Skip to content

Conversation

@katiewasnothere
Copy link

@katiewasnothere katiewasnothere commented Aug 13, 2021

This PR adds a new script Verify-GoModules.ps1 that copies the contents of the repo into a temporary location and runs go mod vendor and go mod tidy in that temporary location. The file hashes of the vendor directories in the temporary location and in the repo's location are compared to determine difference.

The script needs to be run as admin due to requirements of Get-FileHash. As a result, we cannot easily get the error messages from the script.

However, one benefit of using powershell for this is that we can take advantage of the Get-FileHash cmdlet, allowing us to get a more accurate and faster result.

Signed-off-by: Kathryn Baldauf [email protected]

@katiewasnothere katiewasnothere requested a review from a team as a code owner August 13, 2021 18:55
@katiewasnothere
Copy link
Author

@SeanTAllen fyi and thoughts?

@SeanTAllen
Copy link
Contributor

Two thoughts:

  1. Being notified is awesome- I think this is an excellent addition.
  2. Getting a message as part of the output for how one can address the situation to get the PR passing would be very helpful for anyone who isn't a regular contributor. +1 to that idea that Danny mentioned.

My powershell is so poor that I can't comment on anything other than the intent.

@katiewasnothere katiewasnothere force-pushed the verify_modules branch 5 times, most recently from 50f3aea to 5014fa4 Compare August 30, 2021 19:50
@dcantah
Copy link
Contributor

dcantah commented Aug 30, 2021

@katiewasnothere I assume you gave this a spin on your fork?

@katiewasnothere
Copy link
Author

@dcantah I updated the code again. You can see that the test vendor job fails in the checks because those modules are not up to date from previous PRs.

@katiewasnothere
Copy link
Author

Addressed updating the modules in #1141

@dcantah
Copy link
Contributor

dcantah commented Sep 7, 2021

Needs a rebase because of this #1125

@katiewasnothere
Copy link
Author

@dcantah or @anmaxvl is this okay to merge?

@dcantah
Copy link
Contributor

dcantah commented Sep 7, 2021

Go for it

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.

4 participants