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

formulae: set root_url to GitHub Releases url #486

merged 2 commits into from Sep 15, 2020


Copy link

@dawidd6 dawidd6 commented Aug 22, 2020

If not testing core tap (and a bunch of other conditions), set the root_url to GitHub releases url.
This url would later be used in brew pr-upload to determine what release to create.

lib/tests/formulae.rb Outdated Show resolved Hide resolved
Copy link

Bo98 commented Aug 22, 2020

Not entirely sure on the !args.keep_old? case. Doesn't brew bottle mandate root_url is the same between the bottle json and the formula file when --keep-old is passed?

@dawidd6 dawidd6 force-pushed the github-releases branch 2 times, most recently from ec83dcd to 7cb8a23 Compare August 22, 2020 16:49
Copy link

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Makes sense to me (minus @Bo98's question I'd like to see figured out). --ci-upload should probably be configured to fail with a helpful message given this case, too?

Copy link
Member Author

dawidd6 commented Aug 24, 2020

Yes, --keep-old was a bad idea, deleted it.

Also added a check for --ci-upload to refuse uploading to GitHub Releases.

lib/test_ci_upload.rb Outdated Show resolved Hide resolved
lib/test_ci_upload.rb Outdated Show resolved Hide resolved
@dawidd6 dawidd6 merged commit 5a02d7f into Homebrew:master Sep 15, 2020
@dawidd6 dawidd6 deleted the github-releases branch September 15, 2020 11:01
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
None yet

Successfully merging this pull request may close these issues.

None yet

4 participants