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

cleanup: remove db config #709

Merged
merged 17 commits into from
May 23, 2024
Merged

cleanup: remove db config #709

merged 17 commits into from
May 23, 2024

Conversation

aalu1418
Copy link
Collaborator

@aalu1418 aalu1418 commented May 17, 2024

core ref: c00f33248fe41faafaf67315d3c2665ba0656488

downstream:

@aalu1418 aalu1418 requested a review from a team as a code owner May 17, 2024 19:28
@aalu1418 aalu1418 marked this pull request as draft May 17, 2024 19:28
Copy link
Collaborator Author

@aalu1418 aalu1418 left a comment

Choose a reason for hiding this comment

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

overall goal was to remove the old DB configs and reduce duplicate configuration code

pkg/solana/config/toml.go Outdated Show resolved Hide resolved
pkg/solana/config/toml.go Show resolved Hide resolved
pkg/solana/config/toml.go Show resolved Hide resolved
}
return
return errors.Join(errA...)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this deferred join still necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the internal types will superficially be different, but if we are just using Unwrap() []error, then we still see the same slice: https://go.dev/play/p/65QFKJrpODU

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah we are really just using Unwrap() []error but I think everything is passed around via the error interface?

this seems like the simplest way to meet that interface without implementing a new type

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean that there should be no difference. Are you certain this is necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you mean that we depend on always having something that implements Unwrap() []error, then couldn't we can relax that assumption and do a conversion for the base cases of 0/1 on the other side?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

update: this is the only way that works right now - doing a recursive unwrap within core does not solve the issue

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cl-sonarqube-production
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
33.3% Coverage on New Code (required ≥ 75%)

See analysis details on SonarQube

@aalu1418 aalu1418 enabled auto-merge (squash) May 23, 2024 17:41
@aalu1418 aalu1418 merged commit 45db170 into develop May 23, 2024
25 of 26 checks passed
@aalu1418 aalu1418 deleted the cleanup/remove-db-config branch May 23, 2024 17:48
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