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

Xorg: Use PageMatch#find_versions #10113

Merged
merged 3 commits into from Dec 24, 2020
Merged

Xorg: Use PageMatch#find_versions #10113

merged 3 commits into from Dec 24, 2020

Conversation

samford
Copy link
Member

@samford samford commented Dec 23, 2020

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

I'm working on extracting changes in #9535 that aren't specifically related to the migration from open-uri to curl into separate, topical PRs. This will help to minimize the number of changes in that PR, which certainly won't hurt when it comes to troubleshooting the hang we're seeing when using curl.

This PR modifies PageMatch#find_versions to accept an optional provided_content parameter, which allows the method to work with cached page content. When page content is provided, we don't fetch the content at the url (using Strategy#page_content).

Since the Xorg strategy tends to check the same x.org pages over and over for related formulae, we implemented a simple caching system to minimize the number of times we fetch these pages. As a result, Xorg#find_versions duplicates some of the logic found in PageMatch, simply to be able to work with cached page content. The previously-mentioned change to PageMatch allows us to modify Xorg#find_versions to simply use PageMatch#find_versions internally, like many other strategies.

Outside of bringing Xorg more in line with the other strategies, allowing PageMatch#find_versions to work with a content string is a first step towards enabling page caching throughout livecheck.

@BrewTestBot
Copy link
Member

Review period will end on 2020-12-24 at 14:32:47 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Dec 23, 2020
Copy link
Member

@maxim-belkin maxim-belkin left a comment

Choose a reason for hiding this comment

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

👍🏻 Looks good!

@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Dec 24, 2020
@BrewTestBot
Copy link
Member

Review period ended.

@samford samford merged commit 8ac02c9 into Homebrew:master Dec 24, 2020
@samford samford deleted the xorg-use-pagematch branch December 24, 2020 22:59
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Jan 24, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Jan 24, 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

4 participants