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

version: add new Version::Parser class #10378

Merged
merged 2 commits into from Jan 25, 2021

Conversation

SeekingMeaning
Copy link
Contributor

@SeekingMeaning SeekingMeaning commented Jan 20, 2021

  • 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 typecheck 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?

This PR adds four new classes for parsing version strings from Pathnames: Parser, RegexParser, UrlParser, and StemParser

  • Parser is the base abstract class and currently does nothing; all it contains is an abstract parse method.
    • (This class is useful to have if we ever need to parse version strings through means other than regular expressions.)
  • RegexParser is a sub-class of Parser and uses regular expressions to extract version strings from Pathnames.
    • This class is also abstract; sub-classes of RegexParser must implement the process_spec method.
    • It accepts an optional block that is called to finalize the parsed version string; this is used for Boost version styles to replace underscores (_) with dots (.)
  • UrlParser is a sub-class of RegexParser that parses the full Pathname (i.e. URL) string.
  • StemParser is a sub-class of RegexParser that only parses the stem of the Pathname (i.e. the base file name)

@BrewTestBot
Copy link
Member

Review period will end on 2021-01-21 at 22:58:36 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Jan 20, 2021
Library/Homebrew/version.rb Outdated Show resolved Hide resolved
Library/Homebrew/version.rb Outdated Show resolved Hide resolved
@SeekingMeaning SeekingMeaning changed the title version: put regexes in constant version: add new Version::Parser class Jan 21, 2021
@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Jan 22, 2021
@BrewTestBot
Copy link
Member

Review period ended.

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.

Yeh, that's dramatically cleaner, nice work!


class Version
# @api private
class Parser
Copy link
Member

Choose a reason for hiding this comment

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

Any unit tests for these would be lovely but non-blocking.

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.

👏🏻 ❤️

@SeekingMeaning SeekingMeaning merged commit 0a0f435 into Homebrew:master Jan 25, 2021
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Feb 25, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Feb 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants