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

hack: add script to verify receipts migration #255

Merged
merged 1 commit into from
Jul 15, 2019
Merged

hack: add script to verify receipts migration #255

merged 1 commit into from
Jul 15, 2019

Conversation

ahmetb
Copy link
Member

@ahmetb ahmetb commented Jul 12, 2019 •
edited
Loading

This script makes sure as long as we have the 'system receipts-upgrade' command
upgrades from krew v0.2.1 to the currently built version works.

This should be deleted once the 'system receipts-upgrade' command is gone.

/hold
rebase + merge after #249 (tests expected to fail until then)

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 12, 2019
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 12, 2019
@ahmetb ahmetb mentioned this pull request Jul 12, 2019
Merged
@ahmetb ahmetb force-pushed the test_receipts-migration branch from a52b118 to 5893820 Compare July 12, 2019 23:15
@codecov-io

This comment has been minimized.

Sign in to view
@ahmetb ahmetb force-pushed the test_receipts-migration branch from 5893820 to de06577 Compare July 12, 2019 23:20
corneliusweig
corneliusweig suggested changes Jul 13, 2019
View reviewed changes
Copy link
Contributor

@corneliusweig corneliusweig left a comment

Choose a reason for hiding this comment

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

It's a great idea to test the migration in this way!

The script is a thorough piece of art. I could only find small nits.

@ahmetb ahmetb force-pushed the test_receipts-migration branch 2 times, most recently from c498abc to 8f39524 Compare July 14, 2019 04:11
@ahmetb
Copy link
Member Author

ahmetb commented Jul 14, 2019

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 14, 2019
@ahmetb
Copy link
Member Author

ahmetb commented Jul 14, 2019

I couldn't think of any additional test cases, so I threw krew search and krew list in there just to make sure we can read manifest files and read receipts.

corneliusweig
corneliusweig reviewed Jul 15, 2019
View reviewed changes
Copy link
Contributor

@corneliusweig corneliusweig left a comment

Choose a reason for hiding this comment

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

Two more questions, then this can be merged.

@corneliusweig
Copy link
Contributor

/retest

@ahmetb ahmetb force-pushed the test_receipts-migration branch from 8f39524 to aea8842 Compare July 15, 2019 17:17
@ahmetb
hack: add script to verify receipts migration
2479673 
This script makes sure as long as we have the 'system receipts-upgrade' command
upgrades from krew v0.2.1 to the currently built version works.

This should be deleted once the 'system receipts-upgrade' command is gone.

Signed-off-by: Ahmet Alp Balkan <ahmetb@google.com>
@ahmetb ahmetb force-pushed the test_receipts-migration branch from aea8842 to 2479673 Compare July 15, 2019 18:33
@ahmetb
Copy link
Member Author

ahmetb commented Jul 15, 2019

Looks like doing cd inside a function is actually changing the $CWD of the script (since we define function inside a { } which is not a subshell). So started a subshell in install_krew_0_2_1 function now to not put up with side-effects of cd.

@corneliusweig
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 15, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahmetb, corneliusweig

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:
  • OWNERS [ahmetb,corneliusweig]

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

@k8s-ci-robot k8s-ci-robot merged commit 71bc682 into kubernetes-sigs:master Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corneliusweig corneliusweig corneliusweig requested changes

@juanvallejo juanvallejo Awaiting requested review from juanvallejo

Assignees

@corneliusweig corneliusweig

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ahmetb @codecov-io @corneliusweig @k8s-ci-robot