Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore/dependency pruning and maintenance #1110

Merged
merged 3 commits into from
Oct 14, 2024

Conversation

grisha87
Copy link
Contributor

No description provided.

the library was installing the pino-pretty-logger, axios and bottleneck
when not using it at all

some of the dependencies were reallyd
dependencies for the examples and got moved there
@grisha87 grisha87 requested a review from SewerynKras October 14, 2024 09:02
@@ -97,7 +97,7 @@ export class PaymentModuleImpl implements PaymentModule {
constructor(deps: GolemServices, options?: PaymentModuleOptions) {
const network = options?.network ?? this.options.network;
const driver = options?.driver ?? this.options.driver;
const token = options?.token ?? MAINNETS.includes(network) ? "glm" : "tglm";
const token = (options?.token ?? MAINNETS.includes(network)) ? "glm" : "tglm";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you accidentally discovered a bug here! The logic is supposed to be

{
  if (options?.token) return options.token
  return MAINNETS.includes(network) ? "glm" : "tglm"
}

but it's broken (actually, always has been!). Look at this code:

const token: string | undefined = "my-token"
const some_condition = true;
console.log(token ?? some_condition ? "glm" : "tglm"); // "glm"
console.log((token ?? some_condition) ? "glm" : "tglm"); // "glm"

@grisha87 grisha87 requested a review from SewerynKras October 14, 2024 10:45
@SewerynKras SewerynKras merged commit d3826ea into beta Oct 14, 2024
18 checks passed
@SewerynKras SewerynKras deleted the chore/dependency-pruning-and-maintenance branch October 14, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants