Skip to content

Conversation

@cklin
Copy link
Contributor

@cklin cklin commented Jul 14, 2025

This PR strengthens the contract for the restrictAlertsTo and the restrictAlertsToExactLocation extensible predicates.

  • restrictAlertsTo is now documented to match any alert location that intersects with a specified line range. (Previously an alert location matches only when its first line is in a specified line range.)
  • restrictAlertsToExactLocation is now documented to match any alert location that wholly contains a specific character range. (Previously an alert location matchis only when it is exactly the same as a specified character range.)

It also contains misc wording changes for clarity.

@cklin
Copy link
Contributor Author

cklin commented Aug 11, 2025

@jbj @hvitved

This is a documentation update follow-up to #19943, concerning support for approximate locations. (That PR changes restrictAlertsTo diff-range location matching from "match the first line of location only" to "test whether the location intersects with the diffrange".)

#19943 treats support for approximate locations as an implementation detail that is not documented (nor expected to be documented.) This PR treats the same support for approximate locations as the new specification for restrictAlertsTo (and restrictAlertsToExactLocation), so that all other users of diff-informed queries, on both the QL authoring side and the CI integration side, can take advantage of this more general (and, I believe, more useful) behavior.

jbj
jbj previously approved these changes Aug 12, 2025
Copy link
Contributor

@jbj jbj left a comment

Choose a reason for hiding this comment

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

LGTM! It's great to see the documentation shrinking and becoming clearer while the use cases for diff-informed queries expand just by changing the spec to match the implementation.

Cc @asgerf.

@cklin cklin marked this pull request as ready for review August 12, 2025 14:39
Copilot AI review requested due to automatic review settings August 12, 2025 14:39
@cklin cklin added the no-change-note-required This PR does not need a change note label Aug 12, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the documentation for the AlertFiltering module to clarify and strengthen the contract for the restrictAlertsTo and restrictAlertsToExactLocation extensible predicates. The changes modify how alert location matching is defined - restrictAlertsTo now matches locations that intersect with a line range rather than just checking the first line, and restrictAlertsToExactLocation now matches locations that wholly contain a character range rather than requiring exact matches.

  • Updated parameter names and documentation for restrictAlertsTo to clarify line range intersection behavior
  • Modified restrictAlertsToExactLocation documentation to specify character range containment semantics
  • Simplified and clarified various docstring explanations throughout the module

This commit strengthens the contract for the restrictAlertsTo and the
restrictAlertsToExactLocation extensible predicates.

- restrictAlertsTo is now documented to match any alert location that
  intersects with a specified line range. (Previously an alert location
  matches only when its first line is in a specified line range.)

- restrictAlertsToExactLocation is now documented to match any alert
  location that wholly contains a specific character range. (Previously
  an alert location matchis only when it is exactly the same as a
  specified character range.)

It also contains misc wording changes for clarity.
@cklin cklin force-pushed the cklin/alert-filtering-qldoc branch from 7083d81 to b20521b Compare August 12, 2025 14:44
@cklin cklin requested a review from jbj August 12, 2025 19:52
@cklin cklin merged commit 4c263c0 into main Aug 13, 2025
36 checks passed
@cklin cklin deleted the cklin/alert-filtering-qldoc branch August 13, 2025 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants