Skip to content
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

Set glide alias for goautoneg package #23327

Merged
merged 3 commits into from Jul 15, 2019

Conversation

Projects
None yet
5 participants
@cblecker
Copy link
Member

commented Jul 6, 2019

Set a glide repo alias for the bitbucket.org/ww/goautoneg package due to Masterminds/glide#1057.

Upstream previously did this in kubernetes/kubernetes#72138.

Modelled this PR after the latest dependency bump PR (#23281).

cblecker added some commits Jul 6, 2019

@cblecker cblecker changed the title Goautoneg Set glide alias for goautoneg package Jul 6, 2019

@openshift-ci-robot openshift-ci-robot requested review from deads2k and soltysh Jul 6, 2019

@cblecker

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2019

/assign @deads2k

@cblecker

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2019

/test e2e-aws

@cblecker

This comment has been minimized.

Copy link
Member Author

commented Jul 6, 2019

Tests passing. This one is ready for review.

@deads2k

This comment has been minimized.

Copy link
Contributor

commented Jul 8, 2019

/assign @tnozicka

# https://github.com/Masterminds/glide/issues/1057; upstream did this in
# https://github.com/kubernetes/kubernetes/pull/72138
- package: bitbucket.org/ww/goautoneg
repo: https://github.com/munnerz/goautoneg.git

This comment has been minimized.

Copy link
@tnozicka

tnozicka Jul 9, 2019

Contributor

is that what upstream did?

this does the trick without changing the repo location https://github.com/openshift/library-go/pull/469/files#diff-395f41b2a27b70ce44399c91c82158c2R68

This comment has been minimized.

Copy link
@cblecker

cblecker Jul 9, 2019

Author Member

We don't run glide in upstream (went straight from Godeps to go modules) -- we just straight up replaced the dependency with the forked copy. We were also seeing flakiness in bitbucket for CI (but obviously at a much higher PR rate).

We already import the forked dependency too, because of the upstream Kubernetes changes. I think either change is acceptable with a slight preference to move to the github fork to align with upstream.

Let me know if you'd like me to update my PR, @tnozicka :)

This comment has been minimized.

Copy link
@cblecker

cblecker Jul 10, 2019

Author Member

@tnozicka I confirmed that locally, your fix works for openshift/origin as well. Happy to go with either option.

This comment has been minimized.

Copy link
@tnozicka

tnozicka Jul 15, 2019

Contributor

given upstream switched to that fork, both options look ok to me

This comment has been minimized.

Copy link
@tnozicka

tnozicka Jul 15, 2019

Contributor

/unassign
/assign @deads2k

@cblecker

This comment has been minimized.

Copy link
Member Author

commented Jul 12, 2019

ping @tnozicka .. looking for feedback. I'm okay with either fix, but am blocked on getting dependency bumps into origin.

@deads2k

This comment has been minimized.

Copy link
Contributor

commented Jul 15, 2019

/lgtm

@openshift-ci-robot

This comment has been minimized.

Copy link

commented Jul 15, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cblecker, deads2k

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 3700c91 into openshift:master Jul 15, 2019

10 checks passed

Travis CI - Pull Request Build Passed
Details
ci/prow/cmd Job succeeded.
Details
ci/prow/e2e-aws Job succeeded.
Details
ci/prow/e2e-aws-serial Job succeeded.
Details
ci/prow/e2e-aws-upgrade Job succeeded.
Details
ci/prow/images Job succeeded.
Details
ci/prow/integration Job succeeded.
Details
ci/prow/unit Job succeeded.
Details
ci/prow/verify Job succeeded.
Details
tide In merge pool.
Details

@cblecker cblecker deleted the cblecker:goautoneg branch Jul 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.