Skip to content

livecheck: make throttle rate and days mutually exclusive - #24451

Merged
MikeMcQuaid merged 2 commits into
mainfrom
throttle-rate-days-mutually-exclusive
Nov 3, 2026
Merged

livecheck: make throttle rate and days mutually exclusive#24451
MikeMcQuaid merged 2 commits into
mainfrom
throttle-rate-days-mutually-exclusive

Conversation

@cho-m

@cho-m cho-m commented Nov 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

There hasn't been a reasonable scenario where both should be used. Instead it ended up resulting in more issues and complexity when temporarily enabled.

All cases where using both throttles were attempted, it made more sense to use only days throttle.

Removing this avoids having to maintain unused code paths which are checked by AI review: #24450

Not sure if we need a deprecation period here. For core taps, only opencode used it from March 31 - June 13. It was never documented on https://docs.brew.sh/

  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include brew benchmark results.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?
  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

n/a

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a87b098c-7d6f-4541-bb4a-526d6e5fa28b
Commits

Reviewing files that changed from the base of the PR and between 85cc61b and 69b2d1d.

Files selected for processing: 2
  • Library/Homebrew/formula.rb
  • Library/Homebrew/livecheck.rb

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Summary

Summary by CodeRabbit

  • Behaviour Changes
    • Throttle settings now require choosing either a release rate or a minimum number of days between updates. Supplying both options, including when one is already configured, raises an error, except for metadata-loaded non-resource targets. Release-rate and day-based throttling remain available when configured separately.
  • Documentation
    • Clarified that the throttle options are mutually exclusive and that the day setting specifies a minimum wait between updates, rather than serving as a fallback.

Livecheck#throttle now rejects configurations that combine a release rate and a day interval, except for metadata-loaded non-resources. Its documentation describes the options as mutually exclusive and days as a minimum wait. Formula metadata detection and related tests and fixtures are updated.

Changes: Throttle option validation

Layer / File(s) Summary
Throttle argument contract
Library/Homebrew/livecheck.rb, Library/Homebrew/formula.rb, Library/Homebrew/test/livecheck_spec.rb		 Livecheck#throttle documents rate and days as mutually exclusive and raises ArgumentError when supplied or stored options combine both modes, except for metadata-loaded non-resources. Formula.loaded_from_metadata? checks whether the formula namespace directly defines LOADED_FROM_METADATA, and no_autobump! uses this predicate. Specs check the throttle errors.
Throttle coverage updates
Library/Homebrew/test/dev-cmd/bump-cask-pr_spec.rb, Library/Homebrew/test/dev-cmd/bump-formula-pr_spec.rb, Library/Homebrew/test/formula_auditor_spec.rb, Library/Homebrew/test/livecheck/livecheck_spec.rb		 Tests and fixtures for combined rate-and-days throttles are removed. Remaining throttle checks use rate-only or days-only options.
Merge Risk: Minimal · up to 69b2d

No actionable merge-blocking risk is established for this change.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the main change: livecheck throttle rate and throttle days become mutually exclusive. The changeset adds an ArgumentError when both are set, updates the documentation, and removes the …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description directly explains why combined throttle rates and days are being removed and relates to the changes in the pull request.
Full details: Title check

Explanation

The title names the main change: livecheck throttle rate and throttle days become mutually exclusive. The changeset adds an ArgumentError when both are set, updates the documentation, and removes the combined-throttle tests and fixtures. The title is concise and specific.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

coderabbitai[bot]
coderabbitai Bot reviewed Nov 2, 2026
View reviewed changes
Comment thread Library/Homebrew/livecheck.rb Resolved
@cho-m
livecheck: make throttle rate and days mutually exclusive
85cc61b 
There hasn't been a reasonable scenario where both should be used.
Instead it ended up resulting in more issues and complexity when
temporarily enabled.
@cho-m
cho-m force-pushed the throttle-rate-days-mutually-exclusive branch from a25d9d5 to 85cc61b Compare November 2, 2026 22:28
@cho-m cho-m mentioned this pull request Nov 3, 2026
Open
6 of 8 tasks
ZhongRuoyu
ZhongRuoyu approved these changes Nov 3, 2026
View reviewed changes

@ZhongRuoyu ZhongRuoyu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

Don't think it's necessary to go through the deprecation cycle. Plus there doesn't seem to be any public usage: https://github.com/search?q=%2Fthrottle+.%2B%2C+days%3A+.%2B%2F+NOT+is%3Afork&type=code

MikeMcQuaid
MikeMcQuaid approved these changes Nov 3, 2026
View reviewed changes

@MikeMcQuaid MikeMcQuaid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agreed with no deprecation cycle being needed.

Thanks!

@MikeMcQuaid
MikeMcQuaid added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 46fe538 Nov 3, 2026
55 checks passed
@MikeMcQuaid
MikeMcQuaid deleted the throttle-rate-days-mutually-exclusive branch November 3, 2026 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments
@MikeMcQuaid MikeMcQuaid MikeMcQuaid approved these changes
@ZhongRuoyu ZhongRuoyu ZhongRuoyu approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cho-m @MikeMcQuaid @ZhongRuoyu