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

create yank-buildpack command #754

Merged
merged 3 commits into from
Aug 5, 2020
Merged

create yank-buildpack command #754

merged 3 commits into from
Aug 5, 2020

Conversation

elbandito
Copy link
Member

@elbandito elbandito commented Jul 17, 2020

Summary

**NOTE: In the RFC, there is mention of 2 types to support: github and git. This PR is ONLY for supporting github.

This PR introduces the new pack yank-buildpack command (hidden behind the experimental flag). This command is used to update a previously registered buildpack as yanked inside the Github registry . If the user wants to undo a previously yanked buildpack from the registry, they can use the --undo flag. In both cases, the command will open a new browser window to create/submit a Github Issue (see below).

> pack yank-buildpack heroku/rust@1.2.3

image

> pack yank-buildpack heroku/rust@1.2.3 --undo

image

Output

pack yank-buildpack --help
yank the buildpack from the registry

Usage:
  pack yank-buildpack <buildpack-id-and-version> [flags]

Flags:
  -r, --buildpack-registry string   Buildpack Registry name
  -h, --help                        Help for 'yank-buildpack'
  -u, --undo                        undo previously yanked buildpack

Global Flags:
      --no-color     Disable color output
  -q, --quiet        Show less output
      --timestamps   Enable timestamps in output
  -v, --verbose      Show more output

For more details on the flow of this command and how it's used, see the RFC.

Documentation

  • Should this change be documented?
    • Yes, see #___
    • No

Related

Resolves #___

@codecov
Copy link

codecov bot commented Jul 17, 2020

Codecov Report

Merging #754 into main will increase coverage by 0.15%.
The diff coverage is 82.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #754      +/-   ##
==========================================
+ Coverage   73.53%   73.67%   +0.15%     
==========================================
  Files          75       77       +2     
  Lines        5103     5184      +81     
==========================================
+ Hits         3752     3819      +67     
- Misses       1037     1048      +11     
- Partials      314      317       +3     
Flag Coverage Δ
#os_linux 76.15% <84.06%> (+0.13%) ⬆️
#os_macos 72.08% <84.06%> (+0.19%) ⬆️
#os_windows 72.05% <82.76%> (+0.17%) ⬆️
#unit 73.67% <82.76%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@elbandito elbandito marked this pull request as ready for review July 17, 2020 22:35
@elbandito elbandito requested a review from a team as a code owner July 17, 2020 22:35
@elbandito elbandito force-pushed the yank_buildpack branch 2 times, most recently from 5a43b45 to 58fb548 Compare July 21, 2020 05:20
Copy link
Member

@dfreilich dfreilich left a comment

Choose a reason for hiding this comment

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

A few nits, but overall this is great! Definitely appreciate the efforts at testing in this one

internal/commands/yank_buildpack.go Show resolved Hide resolved
internal/commands/yank_buildpack.go Show resolved Hide resolved
internal/commands/yank_buildpack.go Outdated Show resolved Hide resolved
internal/commands/yank_buildpack.go Outdated Show resolved Hide resolved
internal/commands/yank_buildpack_test.go Outdated Show resolved Hide resolved
yank_buildpack.go Outdated Show resolved Hide resolved
yank_buildpack_test.go Show resolved Hide resolved
yank_buildpack.go Show resolved Hide resolved
yank_buildpack.go Show resolved Hide resolved
internal/registry/registry_cache.go Outdated Show resolved Hide resolved
@jromero
Copy link
Member

jromero commented Jul 27, 2020

@elbandito, @dfreilich left a few comments. I'm eager to run through acceptance to get this in but want to wait for those conversations to be resolved so that it doesn't happen in vain if a few things might change. Let me know if I should proceed anyway.

@elbandito
Copy link
Member Author

elbandito commented Jul 27, 2020

@elbandito, @dfreilich left a few comments. I'm eager to run through acceptance to get this in but want to wait for those conversations to be resolved so that it doesn't happen in vain if a few things might change. Let me know if I should proceed anyway.

Thanks for the comments @dfreilich!
@jromero let me make some changes first then I'll give you the heads-up.

@elbandito elbandito force-pushed the yank_buildpack branch 4 times, most recently from 44cb2a9 to fd6493f Compare July 30, 2020 19:00
@elbandito
Copy link
Member Author

@jromero I made some changes. It's now ready for you to run some UA tests.

internal/registry/buildpack.go Outdated Show resolved Hide resolved
Signed-off-by: Travis <longoria.public@gmail.com>
@jromero
Copy link
Member

jromero commented Aug 4, 2020

@elbandito I'm not sure if you've seen some minor unresolved test related comments from @dfreilich (they may be hidden by default in the GitHub UI). Going to start UA, just thought I'd mention it in case they were missed.

@elbandito
Copy link
Member Author

@dfreilich did you have anything else you'd like for me to address before approving? I did pass on the godoc nits for this PR, but happy to look into them later.

@dfreilich
Copy link
Member

@elbandito It generally looks great. If you could add a test or two for GetIssueURL though, it would be nice (though it's simple, it's nice to have it be tested anyways in case it grows more complex)

Signed-off-by: Travis <longoria.public@gmail.com>
@elbandito
Copy link
Member Author

GetIssueURL

@dfreilich sorry, must have missed that comment. The test has been added.

@elbandito elbandito merged commit 408dd8a into main Aug 5, 2020
@elbandito elbandito deleted the yank_buildpack branch August 5, 2020 16:55
@jromero jromero added this to the 0.13.0 milestone Aug 5, 2020
@jromero jromero added the type/enhancement Issue that requests a new feature or improvement. label Aug 5, 2020
@jromero jromero added the experimental Issue or PR refers to an experimental feature. label Aug 5, 2020
@jromero jromero linked an issue Aug 7, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experimental Issue or PR refers to an experimental feature. type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

YANK registered buildpack
3 participants