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

now-cli 16.1.2 (new formula) #41998

Closed
wants to merge 1 commit into from
Closed

now-cli 16.1.2 (new formula) #41998

wants to merge 1 commit into from

Conversation

JounQin
Copy link
Contributor

@JounQin JounQin commented Jul 15, 2019

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

Formula/now-cli.rb Outdated Show resolved Hide resolved
Formula/now-cli.rb Outdated Show resolved Hide resolved
Formula/now-cli.rb Outdated Show resolved Hide resolved
Formula/now-cli.rb Outdated Show resolved Hide resolved
@fxcoudert fxcoudert added the new formula PR adds a new formula to Homebrew/homebrew-core label Jul 17, 2019
@JounQin
Copy link
Contributor Author

JounQin commented Jul 17, 2019

@SMillerDev @fxcoudert Can we merge this PR then?

Formula/now-cli.rb Outdated Show resolved Hide resolved
@JounQin
Copy link
Contributor Author

JounQin commented Jul 25, 2019

Any new review or let's merge it?

@chrmoritz
Copy link
Contributor

chrmoritz commented Jul 25, 2019

@JounQin I don't think your last commit is really necessary, because they are removing the pkg embedding with the next minor (major?) version completely. Then we should be able to just fetch their transpiled files from npm and do the regular normal node formula installation like in: https://gist.github.com/chrmoritz/cf4274d7934e295bd08f4c4e28e9a166#file-now-cli-rb-L11-L18 (using their canary atm, Edit: We still should patch their upgrade command though).

@JounQin
Copy link
Contributor Author

JounQin commented Jul 25, 2019

@JounQin I don't think your last commit is really necessary, because they are removing the pkg embedding with the next minor (major?) version completely. Then we should be able to just fetch their transpiled files from npm and do the regular normal node formula installation like in: https://gist.github.com/chrmoritz/cf4274d7934e295bd08f4c4e28e9a166#file-now-cli-rb-L11-L14 (using their canary atm, Edit: We still should patch their upgrade command though).

You're the genius, I just learned so much from you, thx! I'll reset the last commit after a while.

@SMillerDev
Copy link
Member

can you squash all your commits into one now-cli 15.8.0 (new formula) commit?

@SMillerDev SMillerDev added the almost there PR is nearly ready to merge label Jul 29, 2019
@JounQin
Copy link
Contributor Author

JounQin commented Jul 29, 2019

@SMillerDev GitHub can automatically do that before merging.

@jonchang
Copy link
Contributor

@SMillerDev GitHub can automatically do that before merging.

Yes, but unfortunately our binary workflow is incompatible with the Github "squash and merge" feature. See https://docs.brew.sh/How-To-Open-a-Homebrew-Pull-Request#following-up. Please squash and rebase this pull request, thanks.

@JounQin
Copy link
Contributor Author

JounQin commented Jul 30, 2019

@jonchang done.

@JounQin JounQin changed the title now-cli 15.8.0 (new formula) now-cli 15.8.7 (new formula) Jul 30, 2019
@SMillerDev
Copy link
Member

@BrewTestBot test this please

@JounQin
Copy link
Contributor Author

JounQin commented Jul 31, 2019

What's wrong with @BrewTestBot ?

@chenrui333
Copy link
Member

I think it just needs a reboot.

Formula/now-cli.rb Outdated Show resolved Hide resolved
Formula/now-cli.rb Outdated Show resolved Hide resolved
Formula/now-cli.rb Outdated Show resolved Hide resolved
@JounQin JounQin changed the title now-cli 15.8.7 (new formula) now-cli 16.1.1 (new formula) Aug 16, 2019
@JounQin
Copy link
Contributor Author

JounQin commented Aug 16, 2019

Can we merge this then? It has been a whole month since the PR created. 😂
@jonchang @SMillerDev

@SMillerDev
Copy link
Member

Looks good to me

@SMillerDev SMillerDev added ready to merge PR can be merged once CI is green and removed almost there PR is nearly ready to merge labels Aug 16, 2019
@chrmoritz
Copy link
Contributor

@SMillerDev: Can we get this merged?
@JounQin: now 16.1.2 is available now, so it couldn't hurt if we upgrade to this beforehand

@JounQin JounQin changed the title now-cli 16.1.1 (new formula) now-cli 16.1.2 (new formula) Aug 25, 2019
@JounQin
Copy link
Contributor Author

JounQin commented Aug 25, 2019

Done, can we merge this then?...
@SMillerDev @jonchang

@fxcoudert fxcoudert closed this in d2fe971 Aug 28, 2019
@lock lock bot added the outdated PR was locked due to age label Jan 13, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jan 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new formula PR adds a new formula to Homebrew/homebrew-core outdated PR was locked due to age ready to merge PR can be merged once CI is green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants