8000 chore: export node package errors by TropicalDog17 · Pull Request #3056 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

chore: export node package errors #3056

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 28 commits into from
Jun 11, 2024
Merged

chore: export node package errors #3056

merged 28 commits into from
Jun 11, 2024

Conversation

TropicalDog17
Copy link
Contributor
@TropicalDog17 TropicalDog17 commented May 11, 2024 •
edited
Loading

Partially addresses #1140

PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments
  • Title follows the Conventional Commits spec

@TropicalDog17 TropicalDog17 requested review from a team as code owners May 11, 2024 16:47
@andynog
Copy link
Contributor
andynog commented May 13, 2024

Thanks @TropicalDog17, we will be reviewing the PR

@andynog andynog self-assigned this May 13, 2024
@andynog andynog added the P:integrator-experience Priority: Improve experience for integrators label May 13, 2024
@andynog
Copy link
Contributor
andynog commented May 15, 2024

Hi @TropicalDog17, seems it's failing on linting. Could you please check it ?

For reference: https://github.com/cometbft/cometbft/blob/main/CONTRIBUTING.md#formatting--linting

@TropicalDog17
Copy link
Contributor Author

Hi @TropicalDog17, seems it's failing on linting. Could you please check it ?

For reference: https://github.com/cometbft/cometbft/blob/main/CONTRIBUTING.md#formatting--linting

Thanks @andynog, I've fixed the linting issue and make lint passed

@andynog andynog mentioned this pull request May 24, 2024
Open
20 tasks
andynog
andynog previously requested changes May 24, 2024
View reviewed changes
Copy link
Contributor
@andynog andynog left a comment

Choose a reason for hiding this comment

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

Hi @TropicalDog17, just had a chance to review the PR. I've made some small changes on comments and parameter order. Overall it's good, but in setup.go there are still many errors that were not exported or the exported error was not used, like in line 64

		if err != nil {
			return ChecksummedGenesisDoc{}, fmt.Errorf("couldn't read GenesisDoc file: %w", err)	}

Could you please ensure these other errors in setup.go are also exported ? Thanks !

@andynog andynog added this to the 2024-Q2 milestone May 24, 2024
@TropicalDog17 TropicalDog17 requested a review from andynog June 1, 2024 02:06
melekes
melekes approved these changes Jun 10, 2024
View reviewed changes
Copy link
Contributor
@melekes melekes left a comment

Choose a reason for hiding this comment

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

Thanks @TropicalDog17 ❤️

@melekes melekes enabled auto-merge June 11, 2024 06:35
@melekes melekes dismissed andynog’s stale review June 11, 2024 06:35

the fact that not all errors are exported shouldn't block us from merging this PR

@melekes melekes added this pull request to the merge queue Jun 11, 2024
Merged via the queue into cometbft:main with commit 4b6698b Jun 11, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@melekes melekes melekes approved these changes

@andynog andynog Awaiting requested review from andynog

Assignees

@andynog andynog

Labels
P:integrator-experience Priority: Improve experience for integrators
Projects
No open projects
CometBFT
Status: Done
Milestone
2024-Q2
Development

Successfully merging this pull request may close these issues.
3 participants
@TropicalDog17 @andynog @melekes
0