Skip to content

Only add explicit -mod=vendor if needed #2332

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

Merged
merged 2 commits into from
Jan 10, 2020

Conversation

JAORMX
Copy link
Contributor

@JAORMX JAORMX commented Dec 12, 2019

Description of the change:
-mod=vendor is not needed if it's already set in GOFLAGS. So let's not
set it unless it's needed.

This is an issue for commands such as go test and go vet (
see golang/go#32471 ). This avoids such an
issue.

Motivation for the change:
With the enforcement of GOFLAGS=-mod=vendor in openshift, some projects got broken given that environment variable being set by default and operator-sdk setting that explicitly if the vendor directory exists..

-mod=vendor is not needed if it's already set in GOFLAGS. So lets not
set it unless it's needed.

This is an issue for commands such as `go test` and `go vet` (
see: golang/go#32471 ). This avois such an
issue.
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 12, 2019
Copy link
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

@joelanford @estroz @hasbro17

I think this one requires your look/check.

Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

Two additions then lgtm. @JAORMX thanks for submitting this.

This takes into us the suggestions from @estroz and adds the relevant comments for the addition of the -mod=vendor GOFLAG

Co-Authored-By: Eric Stroczynski <[email protected]>
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 12, 2019
@JAORMX
Copy link
Contributor Author

JAORMX commented Dec 12, 2019

@estroz thanks for the suggestions!

@JAORMX JAORMX requested a review from estroz December 12, 2019 23:20
@camilamacedo86 camilamacedo86 self-requested a review January 8, 2020 00:55
Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 10, 2020
Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

/lgtm

@estroz estroz merged commit 5fc8835 into operator-framework:master Jan 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants