Add checks for dependencies that are not vendored#2596
Merged
thaJeztah merged 1 commit intodocker:masterfrom Jul 6, 2020
Merged
Add checks for dependencies that are not vendored#2596thaJeztah merged 1 commit intodocker:masterfrom
thaJeztah merged 1 commit intodocker:masterfrom
Conversation
thaJeztah
reviewed
Jun 22, 2020
thaJeztah
reviewed
Jun 22, 2020
thaJeztah
requested changes
Jun 25, 2020
Member
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
thanks! left two minor nits
could you also
- squash the commits, so that there's a single commit in this PR
- use a descriptive first line for the commit description (e.g. "Add check for dependencies that are not vendored")
- remove the #2332 from the commit message; I know this is often recommended, but having that reference in the commit message can create a lot of noise on GitHub (if forks of this project merge/cherry-pick the commit etc). Having the reference in the PR description on github is sufficient
Contributor
Author
|
Thanks for the review @thaJeztah. |
thaJeztah
changed the title
Contributor
Author
|
Hey @thaJeztah Wanted to know if there is any other review comment or am I missing anything here? |
silvin-lubecki
approved these changes
Jul 6, 2020
Contributor
silvin-lubecki
left a comment
silvin-lubecki
left a comment
There was a problem hiding this comment.
LGTM, thanks a lot @Madhur1997
`make vendor` fails if any dependency is not vendored. Signed-off-by: Madhur Batra <madhurbatra097@gmail.com>
thaJeztah
approved these changes
Jul 6, 2020
Member
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
whoops, sorry, thought I LGTM'd already 😅
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Madhur Batra madhurbatra097@gmail.com
- What I did
Fix #2332:
make vendornow regards 'WARNING: dependency is not vendored' as anerror and fails when some non vendored dependencies exist.
- How I did it
Added a new validation script that runs against the
vndroutput to find any'WARNING: dependency is not vendored' line. If found script exits with non zero status.
- How to verify it
Added a new import in a go file, and ran go get to install the same in GOPATH.
Now
vndr checkfailed due to unvendored dependency.- Description for the changelog
Enhanced existing Makefile to store the
vndrlogs in a tmp file.Ran a new script against the log to verify that there is no unvendored dependency.
- A picture of a cute animal (not mandatory but encouraged)