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

bump-formula-pr: forward compatibility with `hub fork` #3061

Merged
merged 1 commit into from Sep 8, 2017

Conversation

Projects
None yet
4 participants
@mislav
Contributor

mislav commented Aug 15, 2017

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew tests with your changes locally?

Due to limitations of hub fork in hub 2.2, scripts had to repeat the command at least two times; the 2nd time was to read the fork name from the "fatal: remote MYNAME already exists" message output from git.

In upcoming hub 2.3, the hub fork command is improved to always output the remote name, regardless of whether one already existed or not. With this approach, only one hub fork call will ever be necessary when hub is up to date.

/cc @e-beach github/hub#1535

bump-formula-pr: forward compatibility with `hub fork`
Due to limitations of `hub fork` in hub 2.2, scripts had to repeat the
command at least two times; the 2nd time was to read the fork name from
the "fatal: remote MYNAME already exists" message output from git.

In upcoming hub 2.3, the `hub fork` command is improved to always output
the remote name, regardless of whether one already existed or not. With
this approach, only one `hub fork` call will ever be necessary when hub
is up to date.

@MikeMcQuaid MikeMcQuaid requested a review from ilovezfs Aug 23, 2017

@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Aug 23, 2017

Member

👍 when @ilovezfs has tested it and is happy. Nice work on the feature @mislav!

Member

MikeMcQuaid commented Aug 23, 2017

👍 when @ilovezfs has tested it and is happy. Nice work on the feature @mislav!

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Aug 27, 2017

Contributor

@mislav yes, I'd noticed this. Thanks for taking care of it. Unfortunately, this PR only works for hub stable and HEAD, but devel remains broken (and I'm not sure it's easily fixable since devel outputs nothing). Maybe it's time for a new hub release or at least a new devel release?

Contributor

ilovezfs commented Aug 27, 2017

@mislav yes, I'd noticed this. Thanks for taking care of it. Unfortunately, this PR only works for hub stable and HEAD, but devel remains broken (and I'm not sure it's easily fixable since devel outputs nothing). Maybe it's time for a new hub release or at least a new devel release?

@mislav

This comment has been minimized.

Show comment
Hide comment
@mislav

mislav Aug 28, 2017

Contributor

@ilovezfs Thank you for testing this. Let me check what's up with current devel and get back to you. Yes, it's easy for me to cut a new devel release, but I'd rather that this script works with any version of hub (old or new).

Contributor

mislav commented Aug 28, 2017

@ilovezfs Thank you for testing this. Let me check what's up with current devel and get back to you. Yes, it's easy for me to cut a new devel release, but I'd rather that this script works with any version of hub (old or new).

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Aug 28, 2017

Contributor

Thanks @mislav!

Contributor

ilovezfs commented Aug 28, 2017

Thanks @mislav!

@jasonkarns

This comment has been minimized.

Show comment
Hide comment
@jasonkarns

jasonkarns Sep 7, 2017

Contributor

FWIW, hub --HEAD is now breaks as well:

HOMEBREW_DEVELOPER=true brew bump-formula-pr --url=https://github.com/nodenv/node-build/archive/v2.6.12.tar.gz --sha256=b0983958ad0dcb746a252473ea20ff1bcccbe200a42ed3332345d350d60b8c41 node-build
Already up-to-date.
==> replace "https://github.com/nodenv/node-build/archive/v2.6.11.tar.gz" with "https://github.com/nodenv/node-build/archive/v2.6.12.tar.gz"
==> replace "266170f2a56100125e7a76565ef0796017b3c45891e56db76bd2b233b201194e" with "b0983958ad0dcb746a252473ea20ff1bcccbe200a42ed3332345d350d60b8c41"
M	Formula/node-build.rb
Switched to a new branch 'node-build-2.6.12'
[node-build-2.6.12 65ce9bde1f] node-build 2.6.12
 1 file changed, 2 insertions(+), 2 deletions(-)
Error: cannot get remote from 'hub'!
$ hub --version
git version 2.14.1
hub version 2.3.0-pre10-g2d34517
Contributor

jasonkarns commented Sep 7, 2017

FWIW, hub --HEAD is now breaks as well:

HOMEBREW_DEVELOPER=true brew bump-formula-pr --url=https://github.com/nodenv/node-build/archive/v2.6.12.tar.gz --sha256=b0983958ad0dcb746a252473ea20ff1bcccbe200a42ed3332345d350d60b8c41 node-build
Already up-to-date.
==> replace "https://github.com/nodenv/node-build/archive/v2.6.11.tar.gz" with "https://github.com/nodenv/node-build/archive/v2.6.12.tar.gz"
==> replace "266170f2a56100125e7a76565ef0796017b3c45891e56db76bd2b233b201194e" with "b0983958ad0dcb746a252473ea20ff1bcccbe200a42ed3332345d350d60b8c41"
M	Formula/node-build.rb
Switched to a new branch 'node-build-2.6.12'
[node-build-2.6.12 65ce9bde1f] node-build 2.6.12
 1 file changed, 2 insertions(+), 2 deletions(-)
Error: cannot get remote from 'hub'!
$ hub --version
git version 2.14.1
hub version 2.3.0-pre10-g2d34517
@mislav

This comment has been minimized.

Show comment
Hide comment
@mislav

mislav Sep 8, 2017

Contributor
Contributor

mislav commented Sep 8, 2017

@jasonkarns

This comment has been minimized.

Show comment
Hide comment
@jasonkarns

jasonkarns Sep 8, 2017

Contributor

No biggie. Easy enough to quickly switch back to latest real release before running brew command. Mainly just posting as an FYI that --devel and --HEAD behave the same now.

Contributor

jasonkarns commented Sep 8, 2017

No biggie. Easy enough to quickly switch back to latest real release before running brew command. Mainly just posting as an FYI that --devel and --HEAD behave the same now.

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Sep 8, 2017

Contributor

@jasonkarns HEAD is fine if you pull this PR locally. It hasn't shipped yet.

brew pull https://github.com/Homebrew/brew/pull/3061
Contributor

ilovezfs commented Sep 8, 2017

@jasonkarns HEAD is fine if you pull this PR locally. It hasn't shipped yet.

brew pull https://github.com/Homebrew/brew/pull/3061
@jasonkarns

This comment has been minimized.

Show comment
Hide comment
@jasonkarns

jasonkarns Sep 8, 2017

Contributor

ah, my bad. disregard :(

Contributor

jasonkarns commented Sep 8, 2017

ah, my bad. disregard :(

@MikeMcQuaid MikeMcQuaid merged commit 929edca into Homebrew:master Sep 8, 2017

1 of 3 checks passed

codecov/patch 0% of diff hit (target 66.58%)
Details
codecov/project 66.57% (-0.01%) compared to daa0ea4
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid

MikeMcQuaid Sep 8, 2017

Member

Thanks @mislav! Merging this as-is for now as at least it fixes some cases.

Member

MikeMcQuaid commented Sep 8, 2017

Thanks @mislav! Merging this as-is for now as at least it fixes some cases.

@wesbland wesbland referenced this pull request Feb 9, 2018

Merged

mpich 3.3b1 (devel) #23890

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.