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

Provide indication in the PR body when "go get" updates go.mod #27775

Closed
rarkins opened this issue Mar 7, 2024 · 8 comments · Fixed by #28938
Closed

Provide indication in the PR body when "go get" updates go.mod #27775

rarkins opened this issue Mar 7, 2024 · 8 comments · Fixed by #28938
Assignees
Labels
manager:gomod Go Modules priority-2-high Bugs impacting wide number of users or very important features status:in-progress Someone is working on implementation type:feature Feature (new functionality)

Comments

@rarkins
Copy link
Collaborator

rarkins commented Mar 7, 2024

Describe the proposed change(s).

In the gomod manager, it works like this:

  • Renovate will update one or more lines in the go.mod
  • Renovate calls "go get" to synchronize the go.sum
  • "go get" may update both go.mod and go.sum. Renovate commits both

The above is correct and desirable.

The problem with this is that the PR description of what Renovate intended to update may be less than what was actually updated.

As a minimum, Renovate should display a warning that go.mod was updated by "go get" and for reviewers to check the files.

This could be followed later (by another feature request) a more detailed parsing/summary of what exactly changed.

@rarkins rarkins added type:feature Feature (new functionality) priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others manager:gomod Go Modules labels Mar 7, 2024
@rarkins rarkins added priority-2-high Bugs impacting wide number of users or very important features and removed priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others labels Mar 8, 2024
@rarkins
Copy link
Collaborator Author

rarkins commented Mar 8, 2024

Slight complication: for v0.0.0 digest updates we intend for go to "fix" the modified line for us. So we can't purely flag if the file was modified by go get and need to be a little smarter

@rarkins
Copy link
Collaborator Author

rarkins commented Mar 8, 2024

Also we need to make sure that this warning persist across jobs and isn't reset by the updatePr() command later. We could add a PR comment to make sure, like we do with artifacts errors, but we'd be better to have it in the PR body below the table

@rarkins
Copy link
Collaborator Author

rarkins commented Mar 8, 2024

Here is how to reproduce it in theory:

  • Add dependency A to a go.mod, which has a transitive dependency B
  • Change B to be a direct dependency so that now the go.mod has two direct dependencies - A and B
  • Now wait for A to have an update where it also bumps the version of B
  • Let Renovate update A

What we should see is that Renovate says it's updating only A, but go get will also update B too

@rarkins
Copy link
Collaborator Author

rarkins commented May 3, 2024

Alternative suggestion: return a warning from updateArtifacts, and add a new Artifacts Update Warning comment (similar to we do with errors) so that it persists across runs until another commit is done.

It can say: "Changes to the go.mod file were detected when running go get to update checksums. This may mean that more dependencies may be updated in this PR than is listed in the change table above. Please review the diff of this PR to ensure it's OK".

@zharinov
Copy link
Collaborator

zharinov commented May 6, 2024

Alternative suggestion: return a warning from updateArtifacts, and add a new Artifacts Update Warning comment (similar to we do with errors) so that it persists across runs until another commit is done.

So, this isn't something that is currently implemented?

@rarkins
Copy link
Collaborator Author

rarkins commented May 6, 2024

I think we support errors only and not warnings

@renovate-release
Copy link
Collaborator

🎉 This issue has been resolved in version 37.378.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
manager:gomod Go Modules priority-2-high Bugs impacting wide number of users or very important features status:in-progress Someone is working on implementation type:feature Feature (new functionality)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants