Skip to content

Conversation

@oliverklee
Copy link
Collaborator

Move this method up without any changes to the method or its callers.

We'll clean this up in later changes.

Part of #994.

@oliverklee oliverklee self-assigned this Feb 25, 2025
@oliverklee oliverklee marked this pull request as draft February 25, 2025 10:58
@oliverklee
Copy link
Collaborator Author

Waiting for #991 to be merged first as there will probably be merge conflicts.

@oliverklee oliverklee changed the title [TASK] Move getAllDeclarationBlocks up to CSSBlockList [TASK] Move up getAllDeclarationBlocks to CSSBlockList Feb 25, 2025
@oliverklee oliverklee force-pushed the task/move-all-declaration-blocks branch from 013250c to 3f7a428 Compare February 25, 2025 11:02
@coveralls
Copy link

coveralls commented Feb 25, 2025

Coverage Status

coverage: 54.784%. remained the same
when pulling 5fcbbed on task/move-all-declaration-blocks
into 0642f2b on main.

@oliverklee oliverklee force-pushed the task/move-all-declaration-blocks branch from 3f7a428 to ae0d883 Compare February 26, 2025 07:59
@oliverklee oliverklee requested a review from JakeQZ February 26, 2025 07:59
@oliverklee oliverklee marked this pull request as ready for review February 26, 2025 07:59
Copy link
Collaborator

@JakeQZ JakeQZ left a comment

Choose a reason for hiding this comment

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

The names of the moved test methods don't match those finally agreed upon in #991.

@oliverklee
Copy link
Collaborator Author

oliverklee commented Feb 26, 2025

The names of the moved test methods don't match those finally agreed upon in #991.

This PR here is about getAllDeclarationBlocks. #991 is about getAllRuleSets.

I propose we align the names of the tests for getAllDeclarationBlocks with those for getAllRuleSets in a separate PR.

@JakeQZ
Copy link
Collaborator

JakeQZ commented Feb 26, 2025

I propose we align the names of the tests for getAllDeclarationBlocks with those for getAllRuleSets in a separate PR.

OK. But the getAllRuleSets tests seem to be being removed in this PR.

@oliverklee oliverklee marked this pull request as draft February 26, 2025 10:25
@oliverklee
Copy link
Collaborator Author

But the getAllRuleSets tests seem to be being removed in this PR.

Oh, that shouldn't have happened. Hold on …

Move this method up without any changes to the method or its
callers.

We'll clean this up in later changes.

Part of #994.
@oliverklee oliverklee force-pushed the task/move-all-declaration-blocks branch from ae0d883 to 5fcbbed Compare February 26, 2025 10:28
@oliverklee
Copy link
Collaborator Author

Oh, that shouldn't have happened. Hold on …

Fixed now. Good catch!

@oliverklee oliverklee marked this pull request as ready for review February 26, 2025 10:29
@oliverklee oliverklee requested a review from JakeQZ February 26, 2025 10:29
Copy link
Collaborator

@JakeQZ JakeQZ left a comment

Choose a reason for hiding this comment

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

This PR here is about getAllDeclarationBlocks. #991 is about getAllRuleSets.

I originally thought that the tests had been renamed from #991, since I was looking at the removed getAllRuleSets tests and failed to spot that the moved tests were those for getAllDeclarationBlocks.

I've checked more thoroughly now and confirm that the only difference in the moved tests is the subject now being a ConcreteCSSBlockList rather than a Document.

@JakeQZ JakeQZ merged commit a1f9f58 into main Feb 26, 2025
21 checks passed
@JakeQZ JakeQZ deleted the task/move-all-declaration-blocks branch February 26, 2025 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants