Skip to content

Add support for updating vendor#341

Merged
mtojek merged 3 commits intoelastic:masterfrom
mtojek:update-vendor-1
Apr 14, 2020
Merged

Add support for updating vendor#341
mtojek merged 3 commits intoelastic:masterfrom
mtojek:update-vendor-1

Conversation

@mtojek
Copy link
Copy Markdown
Contributor

@mtojek mtojek commented Apr 14, 2020

This PR extends the mage command with support for refreshing the vendor directory.

@mtojek mtojek requested a review from ruflin April 14, 2020 11:00
@mtojek mtojek self-assigned this Apr 14, 2020
Copy link
Copy Markdown
Collaborator

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

I expected CI to fail because of the junit part but it seems it doesn't 🤔 So I'm good with getting this in and then see what @andrewkroh thinks and perhaps fix it later :-D

github.com/elastic/go-ucfg v0.7.0/go.mod h1:iaiY0NBIYeasNgycLyTvhJftQlQEUO2hpF+FX0JKxzo=
github.com/gorilla/mux v1.7.2 h1:zoNxOV7WjqXptQOVngLmcSQgXmgk4NMz1HibBchjl/I=
github.com/gorilla/mux v1.7.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs=
github.com/jstemmer/go-junit-report v0.9.1 h1:6QPYqodiu3GuPL+7mfx+NwDdp2eTkp9IfEUpgAwUN0o=
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we need these for Jenkins. @andrewkroh might know more here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I updated the code to add the go-junit-report dependency after go mod tidy.

func Vendor() error {
fmt.Println(">> mod - updating vendor directory")

err := sh.RunV("go", "mod", "tidy")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Perhaps we can remove tidy in this PR so it doesn't remove the go-junit part?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I solved this, see above.

@ruflin
Copy link
Copy Markdown
Collaborator

ruflin commented Apr 14, 2020

@mtojek Hm, I perhaps just found a problem with this. Will this create a diff every time a dependency is updated? Meaning a PR could pass CI today but fails tomorrow? This should not be the case.

@mtojek
Copy link
Copy Markdown
Contributor Author

mtojek commented Apr 14, 2020

@mtojek Hm, I perhaps just found a problem with this. Will this create a diff every time a dependency is updated? Meaning a PR could pass CI today but fails tomorrow? This should not be the case.

The dependency graph can be updated with the go get command. The command is not executed by mage check, so the PR won't be updated automatically while bulding in CI. Also, there is a vendor directory to prevent the original sources from disappearing/changing.

Please correct me if I'm wrong.

Copy link
Copy Markdown
Collaborator

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

SGTM. Lets get this in! Thanks for the fix.

@mtojek mtojek merged commit c1597ce into elastic:master Apr 14, 2020
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.

2 participants