Skip to content

[TASK] Add separate methods for RuleSet::removeRule(), with deprecation #1249

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 4, 2025
Merged

[TASK] Add separate methods for RuleSet::removeRule(), with deprecation #1249

merged 4 commits into from
May 4, 2025

Conversation

JakeQZ
Copy link
Collaborator

@JakeQZ JakeQZ commented May 2, 2025 •
edited
Loading

Passing string or null to removeRule() is deprecated. The new methods handle that functionality.

Relates to #1247.

@JakeQZ JakeQZ added cleanup deprecation A method, property, or some functionality has been deprecated. refactor For PRs that refactor code without changing functionality to-backport labels May 2, 2025
@JakeQZ JakeQZ requested review from sabberworm and oliverklee May 2, 2025 00:30
@JakeQZ JakeQZ self-assigned this May 2, 2025
@coveralls
Copy link

coveralls commented May 2, 2025 •
edited
Loading

Coverage Status

coverage: 56.902% (+0.6%) from 56.257%
when pulling eab3658 on task/removerule/split-and-deprecate
into f6b5cd3 on main.

@JakeQZ JakeQZ force-pushed the task/removerule/split-and-deprecate branch from 171e5ee to 4467362 Compare May 2, 2025 01:20
oliverklee
oliverklee requested changes May 2, 2025
View reviewed changes
Copy link
Collaborator

@oliverklee oliverklee left a comment

Choose a reason for hiding this comment

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

The new method(s) also should get tests.

@JakeQZ JakeQZ force-pushed the task/removerule/split-and-deprecate branch from 1c92d96 to d312040 Compare May 2, 2025 22:35
JakeQZ added 2 commits May 2, 2025 23:39
@JakeQZ
[TASK] Add RuleSet::removeMatchingRules(), with deprecation
5109c76 
Passing `string` or `null` to `removeRule()` is deprecated.
The new method handles that functionality.

Relates to #1247.
@JakeQZ
Clarify changelog 'Added' entry
0352420
@JakeQZ JakeQZ force-pushed the task/removerule/split-and-deprecate branch from d312040 to 0352420 Compare May 2, 2025 22:40
@JakeQZ JakeQZ changed the title [TASK] Add RuleSet::removeMatchingRules(), with deprecation [TASK] Add separate methods for RuleSet::removeRule(), with deprecation May 2, 2025
@JakeQZ
Copy link
Collaborator Author

JakeQZ commented May 2, 2025

The new method(s) also should get tests.

I was planning to move the existing tests to the new method when the deprecated API was removed. But you're right, they should be added now, and maybe can be more 'unit'.

JakeQZ added 2 commits May 3, 2025 00:20
@JakeQZ
Add removeAllRules() method
b15a3ab
@JakeQZ
Add tests for the new methods
eab3658 
For consistency with current nomenclature,
`Rule` is used in the test method names,
despite that they are now called 'declarations'.
(Renaming to match the current spec is a separate project.)
@JakeQZ JakeQZ force-pushed the task/removerule/split-and-deprecate branch from 9ed08c9 to eab3658 Compare May 3, 2025 23:31
@JakeQZ
Copy link
Collaborator Author

JakeQZ commented May 3, 2025

Suggested changes made...

@JakeQZ JakeQZ requested a review from oliverklee May 3, 2025 23:39
@JakeQZ
Copy link
Collaborator Author

JakeQZ commented May 3, 2025

Note that I changed the PR title and OP message.

I was unable to do the same for the OC (original commit) due to subsequent commits. Could you update the final commit message for main accordingly when you squash and merge?

@oliverklee oliverklee merged commit 807a11b into main May 4, 2025
21 checks passed
@oliverklee oliverklee deleted the task/removerule/split-and-deprecate branch May 4, 2025 08:25
JakeQZ added a commit that referenced this pull request May 4, 2025
@JakeQZ
[TASK] Add separate methods for RuleSet::removeRule(), with depreca…
efd0d92 
…tion (#1249)

Passing `string` or `null` to `removeRule()` is deprecated.
The new methods handle that functionality.

Relates to #1247.

This is the backport of #1249.
@JakeQZ JakeQZ mentioned this pull request May 4, 2025
Merged
JakeQZ added a commit that referenced this pull request May 4, 2025
@JakeQZ
[TASK] Add separate methods for RuleSet::removeRule(), with depreca…
5e7aa7c 
…tion

Passing `string` or `null` to `removeRule()` is deprecated.
The new methods handle that functionality.

Relates to #1247.

This is the backport of #1249.
JakeQZ added a commit that referenced this pull request May 4, 2025
@JakeQZ
[TASK] Add separate methods for RuleSet::removeRule(), with depreca…
308e366 
…tion

Passing `string` or `null` to `removeRule()` is deprecated.
The new methods handle that functionality.

Relates to #1247.

This is the backport of #1249.
JakeQZ added a commit that referenced this pull request May 4, 2025
@JakeQZ
[TASK] Add separate methods for RuleSet::removeRule(), with depreca…
725de58 
…tion

Passing `string` or `null` to `removeRule()` is deprecated.
The new methods handle that functionality.

Relates to #1247.

This is the backport of #1249.
JakeQZ added a commit that referenced this pull request May 4, 2025
@JakeQZ
[TASK] Add separate methods for RuleSet::removeRule(), with depreca…
abf4387 
…tion

Passing a `string` or `null` to `removeRule()` is deprecated.
The new methods handle that functionality.

Relates to #1247.

This is the backport of #1249 (and some minor dependent changes).
JakeQZ added a commit that referenced this pull request May 4, 2025
@JakeQZ
[TASK] Add separate methods for RuleSet::removeRule(), with depreca…
f87eb61 
…tion

Passing a `string` or `null` to `removeRule()` is deprecated.
The new methods handle that functionality.

Relates to #1247.

This is the backport of #1249 (and some minor dependent changes).
JakeQZ added a commit that referenced this pull request May 4, 2025
@JakeQZ
[TASK] Add separate methods for RuleSet::removeRule(), with depreca…
845e6ab 
…tion

Passing a `string` or `null` to `removeRule()` is deprecated.
The new methods handle that functionality.

Relates to #1247.

This is the backport of #1249 (and some minor dependent changes).
JakeQZ added a commit that referenced this pull request May 4, 2025
@JakeQZ
[TASK] Add separate methods for RuleSet::removeRule(), with depreca…
fbdf55c 
…tion

Passing a `string` or `null` to `removeRule()` is deprecated.
The new methods handle that functionality.

Relates to #1247.

This is the backport of #1249 (and some minor dependent changes).
JakeQZ added a commit that referenced this pull request May 4, 2025
@JakeQZ
[TASK] Add separate methods for RuleSet::removeRule(), with depreca…
726967e 
…tion

Passing a `string` or `null` to `removeRule()` is deprecated.
The new methods handle that functionality.

Relates to #1247.

This is the backport of #1249 (and some minor dependent changes).
JakeQZ added a commit that referenced this pull request May 4, 2025
@JakeQZ
[TASK] Add separate methods for RuleSet::removeRule(), with depreca…
b94cd96 
…tion

Passing a `string` or `null` to `removeRule()` is deprecated.
The new methods handle that functionality.

Relates to #1247.

This is the backport of #1249 (and some minor dependent changes).
JakeQZ added a commit that referenced this pull request May 4, 2025
@JakeQZ
[TASK] Add separate methods for RuleSet::removeRule(), with depreca…
c329373 
…tion

Passing a `string` or `null` to `removeRule()` is deprecated.
The new methods handle that functionality.

Relates to #1247.

This is the backport of #1249.
oliverklee pushed a commit that referenced this pull request May 5, 2025
@JakeQZ
[TASK] Add separate methods for RuleSet::removeRule(), with depreca…
a14dc83 
…tion (#1251)

Passing a `string` or `null` to `removeRule()` is deprecated.
The new methods handle that functionality.

Relates to #1247.

This is the backport of #1249.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliverklee oliverklee oliverklee approved these changes

@sabberworm sabberworm Awaiting requested review from sabberworm

Assignees

@JakeQZ JakeQZ

Labels
cleanup deprecation A method, property, or some functionality has been deprecated. refactor For PRs that refactor code without changing functionality to-backport
Projects
None yet
Milestone
8.9.0: Deprecations and bug fixes
Development

Successfully merging this pull request may close these issues.
3 participants
@JakeQZ @coveralls @oliverklee