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

Ignore go.work.sum #4081

Merged
merged 3 commits into from
May 10, 2024
Merged

Ignore go.work.sum #4081

merged 3 commits into from
May 10, 2024

Conversation

katexochen
Copy link
Contributor

Reasons for making this change:

As the go.work file is excluded, the go.work.sum file, which is generated based on the go.work file, should be excluded, too.
Also mentioned in fbc053f#commitcomment-73352144

Links to documentation supporting these rule changes:
Currently there isn't much documentation regarding go.work.sum (see golang/go#51941), the only thing I can refer to is the proposal, which suggests to not check in the go.work file.

@katexochen
Copy link
Contributor Author

To further support these changes, here are some examples for popular repos ignoring go.work.sum:
hashicorp/nomad#13193
gravitational/teleport#12098
https://github.com/sourcegraph/sourcegraph/pull/36049

Copy link

@avelino avelino left a comment

Choose a reason for hiding this comment

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

LGTM

These files shouldn't be commited since they are specific to a local dev environment.

@StLeoX
Copy link

StLeoX commented Jul 26, 2022

Why not **go.sum?
The sum file is used to check locally, I think.

@katexochen
Copy link
Contributor Author

Why not **go.sum? The sum file is used to check locally, I think.

Typically your module's go.sum file should be committed along with your go.mod file.

@rubenpoppe
Copy link

@avelino, @Nirusu will this get merged?

@smoyer64
Copy link

I'm in favor of this change - if go.work is ignored, then go.work.sum should also be ignored. Note that there's not actually a recommendation or even consensus on whether these files should be ignored. My read on the situation is that for cases like a monorepo, you probably do want to commit these files but in many other situations, you do not. See the discussion at golang/go#53502.

@goto1134
Copy link

@bdougie, could you please take a look at this PR since you already merged #3884? The file mentioned here is a checksum file that is autogenerated, and present iif go.work file is present in the local repository. Since go.work is ignored, we should ignore go.work.sum files as well.

@Omarabdul3ziz
Copy link

Hey there, I noticed that the PR is already approved. is there an estimated timeframe for when it might be merged? Just curious and looking forward to it

@smoyer64
Copy link

I think those gray check marks indicate that the approvals are by developers who are not CODEOWNERS. I don't even see a merge button!

Copy link
Collaborator

@wirecat wirecat left a comment

Choose a reason for hiding this comment

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

Thank you for following our contribution guide and helping improve our templates!
I just finished reviewing and this looks good to me.

@wirecat wirecat merged commit 7b22f8a into github:main May 10, 2024
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.

9 participants