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.

Sign up for GitHub

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

Jump to bottom

Running dep ensure to add required dependencies. #79

Merged
merged 1 commit into from
Sep 28, 2018
Merged

Running dep ensure to add required dependencies. #79

merged 1 commit into from
Sep 28, 2018

Conversation

tejal29
Copy link
Contributor

@tejal29 tejal29 commented Sep 28, 2018 •
edited
Loading

Running dep ensure on master

In commit 2a03869, I ran dep prune
which did not respect the [[prune.projects]] and removed
"github.com/knative/test-infra"
Running dep ensure on master fixes this issue.

@tejal29
Running dep ensure on master
2f43594 
In commit 2a03869, I ran `dep prune`
which did not respect the [[prune.projects]] and removed
"github.com/knative/test-infra"
Running dep ensure on master fixes this issue.
@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 28, 2018
@knative-prow-robot knative-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 28, 2018
@tejal29 tejal29 changed the title Running dep ensure on master Running dep ensure to pick up required dependencies. Sep 28, 2018
@tejal29 tejal29 changed the title Running dep ensure to pick up required dependencies. Running dep ensure to add required dependencies. Sep 28, 2018
@bobcatfish
Copy link
Collaborator

Can you explain more about why dep prune removed things which were needed? Do we need to investigate that further?

/approve
/lgtm
/hold

(Remove /hold when ready with /hold cancel)

@knative-prow-robot knative-prow-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Sep 28, 2018
@knative-prow-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bobcatfish, tejal29

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

@tejal29
Copy link
Contributor Author

tejal29 commented Sep 28, 2018

yeah, its bizzare,
Running dep prune says:

tejaldesai@tejaldesai~/go/src/github.com/knative/build-pipeline: (fix_hack)$ dep prune
Pruning is now performed automatically by dep ensure.
Set prune settings in Gopkg.toml and it it will be applied when running ensure.

This command currently still prunes as it always has, to ease the transition.
However, it will be removed in a future version of dep.

Now is the time to update your Gopkg.toml and remove `dep prune` from any scripts.

And then goes and deletes required files anyways

tejaldesai@tejaldesai~/go/src/github.com/knative/build-pipeline: (fix_hack)$ git status | grep 'test-infra'
	deleted:    vendor/github.com/knative/test-infra/scripts/README.md
	deleted:    vendor/github.com/knative/test-infra/scripts/e2e-tests.sh
	deleted:    vendor/github.com/knative/test-infra/scripts/library.sh
	deleted:    vendor/github.com/knative/test-infra/scripts/presubmit-tests.sh
	deleted:    vendor/github.com/knative/test-infra/scripts/release.sh
	vendor/github.com/knative/test-infra/CONTRIBUTING.md
	vendor/github.com/knative/test-infra/LICENSE
	vendor/github.com/knative/test-infra/OWNERS
	vendor/github.com/knative/test-infra/README.md
	vendor/github.com/knative/test-infra/WORKSPACE
	vendor/github.com/knative/test-infra/dummy.go

I will take a look later.

@tejal29
Copy link
Contributor Author

tejal29 commented Sep 28, 2018 •
edited
Loading

/remove hold
dep prune is deprecated and should be avoided as per this thread
golang/dep#1820 (comment)

@tejal29
Copy link
Contributor Author

tejal29 commented Sep 28, 2018

/hold cancel

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 28, 2018
@knative-prow-robot knative-prow-robot merged commit ff6d5e4 into tektoncd:master Sep 28, 2018
chmouel pushed a commit to chmouel/tektoncd-pipeline that referenced this pull request Jul 17, 2019
@openshift-merge-robot
Merge pull request tektoncd#79 from chmouel/improve-e2e-script-robust…
5b2192c 
…ness

Run E2E script with -e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bobcatfish bobcatfish Awaiting requested review from bobcatfish

@evankanderson evankanderson Awaiting requested review from evankanderson

@jonjohnsonjr jonjohnsonjr Awaiting requested review from jonjohnsonjr

@mattmoor mattmoor Awaiting requested review from mattmoor

@mdemirhan mdemirhan Awaiting requested review from mdemirhan

@shashwathi shashwathi Awaiting requested review from shashwathi

@steuhs steuhs Awaiting requested review from steuhs

@vaikas vaikas Awaiting requested review from vaikas

@yanweiguo yanweiguo Awaiting requested review from yanweiguo

Assignees

@bobcatfish bobcatfish

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tejal29 @bobcatfish @knative-prow-robot