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

Bump common and fix Chain Writer naming #328

Merged
merged 1 commit into from
Dec 4, 2024
Merged

Bump common and fix Chain Writer naming #328

merged 1 commit into from
Dec 4, 2024

Conversation

ilija42
Copy link
Collaborator

@ilija42 ilija42 commented Nov 26, 2024

core ref: fa346afd62ef6f6f97b18ddae4eee2298ebc5001

jmank88
jmank88 previously approved these changes Nov 27, 2024
@ilija42 ilija42 marked this pull request as ready for review December 2, 2024 19:38
@ilija42 ilija42 requested a review from a team as a code owner December 2, 2024 19:38
@jmank88
Copy link
Collaborator

jmank88 commented Dec 2, 2024

Did CI get stuck?

@jmank88
Copy link
Collaborator

jmank88 commented Dec 2, 2024

Did CI get stuck?

Oh, or are the required CI jobs hard-coded with a Go version 1.22 in the name?

@ilija42
Copy link
Collaborator Author

ilija42 commented Dec 2, 2024

Did CI get stuck?

Oh, or are the required CI jobs hard-coded with a Go version 1.22 in the name?

seems like it

winder
winder previously approved these changes Dec 3, 2024
Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

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

LGTM. I believe CI will pass after #339 is merged.

Comment on lines +34 to +37
gosec:
# TODO - remove this and fix linter complaint
excludes:
- G115
Copy link
Collaborator

Choose a reason for hiding this comment

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

How many were there? It would be a little better to mark them directly as //nolint:gosec // G115 .... if it's not too much trouble.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

10+


ensure_golangcilint_1_59:
@golangci-lint --version | grep -q '1.59' || (echo "Please use golangci-lint 1.59" && exit 1)
ensure_golangcilint_1_62_2:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this pattern serve a purpose? Or could we drop it?

Suggested change
ensure_golangcilint_1_62_2:
ensure_golangcilint_version:

Copy link
Contributor

Choose a reason for hiding this comment

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

should drop it

Copy link

github-actions bot commented Dec 4, 2024

Metric cw-rename main
Coverage 74.0% 73.9%

@ilija42 ilija42 merged commit 8956bb6 into main Dec 4, 2024
4 checks passed
@winder winder deleted the cw-rename branch December 4, 2024 03:23
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.

3 participants