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

version: support post versions #8850

Merged
merged 2 commits into from Oct 5, 2020
Merged

version: support post versions #8850

merged 2 commits into from Oct 5, 2020

Conversation

dtrodrigues
Copy link
Member

  • 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 style with your changes locally?
  • Have you successfully run brew tests with your changes locally?
  • Have you successfully run brew man locally and committed any changes?

Add support for PEP 440 post versions. Relates to #8832. In support of Homebrew/homebrew-core#61861 so the version number does not need to be manually specified.

@dtrodrigues
Copy link
Member Author

Not sure what to do here. This PR makes the explicit version line in legit.rb (https://github.com/Homebrew/homebrew-core/blob/092220c889944cd078f6a7416f9580b19e34e699/Formula/legit.rb#L7) redundant and causes testing on this change to fail, but changing that line in legit before another tagged brew version probably wouldn't work either.

MikeMcQuaid
MikeMcQuaid approved these changes Oct 5, 2020
View reviewed changes
Copy link
Member

@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.

👍🏻 once CI is green.

@MikeMcQuaid
Copy link
Member

Not sure what to do here. This PR makes the explicit version line in legit.rb (https://github.com/Homebrew/homebrew-core/blob/092220c889944cd078f6a7416f9580b19e34e699/Formula/legit.rb#L7) redundant and causes testing on this change to fail, but changing that line in legit before another tagged brew version probably wouldn't work either.

I'm not sure what best to do, either. Could just override the required step here and merge the two PRs in quick succession before a tag?

@reitermarkus
Copy link
Member

reitermarkus commented Oct 5, 2020 •
edited

Not sure what to do here.

Add an exception for the formula to the audit/cop and remove it once it is fixed?

@dtrodrigues dtrodrigues mentioned this pull request Oct 5, 2020
Merged
5 tasks
@dtrodrigues
Copy link
Member Author

I submitted a PR for legit (Homebrew/homebrew-core#62121). Merging both quickly and then tagging should work, but is a little risky. A temporary allowlist for redundant versions until the next tag should work, but the challenge is trying to balance doing it "the right way" without requiring too much throwaway work.

@reitermarkus
Copy link
Member

Why is a new tag necessary? homebrew-core CI runs on master anyways, right?

@MikeMcQuaid
Copy link
Member

Why is a new tag necessary? homebrew-core CI runs on master anyways, right?

@reitermarkus It'll break installation for anyone installing not yet on master.

@MikeMcQuaid MikeMcQuaid merged commit 23e24c6 into Homebrew:master Oct 5, 2020
@MikeMcQuaid
Copy link
Member

Thanks @dtrodrigues!

MikeMcQuaid
MikeMcQuaid reviewed Oct 5, 2020
View reviewed changes
Library/Homebrew/dev-cmd/audit.rb
@@ -1024,7 +1024,7 @@ def audit_version
elsif !version.detected_from_url?
version_text = version
version_url = Version.detect(url, **specs)
if version_url.to_s == version_text.to_s && version.instance_of?(Version)
if version_url.to_s == version_text.to_s && version.instance_of?(Version) && @name != "legit"
Copy link
Member

Choose a reason for hiding this comment

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

&& @name != "legit" can now be removed because I've just tagged 2.5.3.

@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 3, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MikeMcQuaid MikeMcQuaid MikeMcQuaid approved these changes

Assignees
No one assigned
Labels
outdated PR was locked due to age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@dtrodrigues @MikeMcQuaid @reitermarkus @BrewTestBot