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

idrive: update livecheck, use versioned url #178648

Merged
merged 2 commits into from
Jul 5, 2024

Conversation

khipp
Copy link
Member

@khipp khipp commented Jul 5, 2024

Important: Do not tick a checkbox if you haven’t performed its action. Honesty is indispensable for a smooth review process.

In the following questions <cask> is the token of the cask you're submitting.

After making any changes to a cask, existing or new, verify:

Additionally, if adding a new cask:

  • Named the cask according to the token reference.
  • Checked the cask was not already refused (add your cask's name to the end of the search field).
  • brew audit --cask --new <cask> worked successfully.
  • HOMEBREW_NO_INSTALL_FROM_API=1 brew install --cask <cask> worked successfully.
  • brew uninstall --cask <cask> worked successfully.

Related to #171006.

@p-linnane p-linnane requested a review from samford July 5, 2024 18:36
Copy link
Member

@samford samford left a comment

Choose a reason for hiding this comment

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

I pushed a commit to make some tweaks to the strategy block:

  • The version and download_id variables are MatchData objects, so it helps to use a variable name that includes "match" to help make this clear (usually we just use a variable named "match" when there's only one match object but we have to distinguish between the two here).
  • I assume that the second part of the version (070624) is numeric or alphanumeric but there's something to be said for using the loose ([^/]+?) instead, which will match any text between the forward slashes.

Outside of that, this looks good to me but someone else will have to approve it since I'm the last pusher.

@samford samford added the ready to merge PR can be merged once CI is green label Jul 5, 2024
@khipp
Copy link
Member Author

khipp commented Jul 5, 2024

@samford Thank you! The advice is much appreciated.

@chenrui333 chenrui333 merged commit a432ec2 into Homebrew:master Jul 5, 2024
11 checks passed
@chenrui333 chenrui333 added the livecheck Issues or PRs related to livecheck label Jul 5, 2024
@khipp khipp deleted the update-idrive branch July 5, 2024 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
livecheck Issues or PRs related to livecheck ready to merge PR can be merged once CI is green
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants