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

build: simplify go mod tidy check #31266

Merged
merged 5 commits into from
Feb 27, 2025

Conversation

islishude
Copy link
Contributor

@islishude
Copy link
Contributor Author

I think we can just use the go mod tidy -diff in the ci scripts instead and remove it from build/ci.go.

that would much better.

@fjl
Copy link
Contributor

fjl commented Feb 26, 2025

I agree it's better to use go mod tidy -diff but it should be called from within ci.go.

One thing we can do, is fold the check into the generated files check. go.mod/go.sum are kind of like generated files anyway.

build/ci.go Outdated
if updates := build.DiffHashes(hashes, tidied); len(updates) > 0 {
log.Fatalf("files changed on running 'go mod tidy': %v", updates)
}
build.MustRun(new(build.GoToolchain).Go("mod", "tidy", "-diff"))
Copy link
Member

Choose a reason for hiding this comment

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

The diff will be printed on the Stdout and Stderr, but the output is not checked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It exits with a non-zero code if the diff is not empty.

@fjl fjl changed the title build/ci: simplify doCheckTidy by using -diff flag build: simplify go mod tidy check Feb 27, 2025
@fjl fjl added this to the 1.15.4 milestone Feb 27, 2025
@fjl fjl merged commit f005d95 into ethereum:master Feb 27, 2025
3 of 4 checks passed
@islishude islishude deleted the build-go-mod-tidy-check branch February 27, 2025 14:52
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