Skip to content
This repository has been archived by the owner on Nov 19, 2024. It is now read-only.

Add CI check to ensure module definition is correct #198

Merged
merged 13 commits into from
Nov 29, 2019

Conversation

nmiyake
Copy link
Contributor

@nmiyake nmiyake commented Nov 25, 2019

Fail verification if "go mod tidy" results in any modifications
to go.mod or go.sum.

Fail verification if "go mod tidy" results in any modifications
to go.mod or go.sum.
@nmiyake
Copy link
Contributor Author

nmiyake commented Nov 25, 2019

This ensures that "go mod tidy" has been run before code has been pushed by failing CI verification if "go mod tidy" causes any modifications to "go.mod" or "go.sum". This would have caught the issue introduced by 14b2737.

The CI task for this PR currently fails this new check due to the commit referenced above -- it should be resolved once #197 is merged and this PR is updated to include it.

This manual diff after running "go mod tidy" is the best that can be done until golang/go#27005 is implemented/available.

Copy link
Owner

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Cool idea, thanks! We'll try it out.

@mholt
Copy link
Owner

mholt commented Nov 25, 2019

Should we test this by merging master into this branch now that #197 is merged?

@nmiyake
Copy link
Contributor Author

nmiyake commented Nov 25, 2019

Interesting, it looks like the redirect diff command (diff -u <(echo -n) <(git diff go.mod)) doesn't work properly for the Windows image. I'm not very familiar with that environment... Options are either:

  1. Figure out way to perform this operation on Windows
  2. Update CI configuration to skip/ignore this operation on Windows (the go mod tidy operation works over all build tags, so the execution environment doesn't matter and the Mac/Linux coverage should probably be sufficient)

(1) is the more "correct" approach, but will take longer since I'll have to iterate against this Azure environment and will mostly just be Googling and trying different things until I can get it to work :)

@nmiyake
Copy link
Contributor Author

nmiyake commented Nov 25, 2019

OK, got this to work on the Windows image as well with only modifications to the CI file -- should be ready.

@mholt mholt merged commit 44285f7 into mholt:master Nov 29, 2019
@mholt
Copy link
Owner

mholt commented Nov 29, 2019

Awesome, thanks for the effort! 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants