Skip to content

[CLEANUP] Extract CSSFunction methods parseName and parseArguments #599

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

Merged
merged 1 commit into from
Jun 22, 2024

Conversation

JakeQZ
Copy link
Contributor

@JakeQZ JakeQZ commented Jun 20, 2024

Also

  • Update the parse function signature to use PHP7 type hints;
  • Remove subsequently redundant DocBlock entries;
  • Ditto for same-named method in subclass to avoid contravariance.

This is the refactoring change from #390.

@JakeQZ JakeQZ added cleanup refactor For PRs that refactor code without changing functionality labels Jun 20, 2024
@JakeQZ JakeQZ added this to the 8.5.2: Bug fixes milestone Jun 20, 2024
@JakeQZ JakeQZ requested review from sabberworm and oliverklee June 20, 2024 22:40
@JakeQZ JakeQZ self-assigned this Jun 20, 2024
@JakeQZ JakeQZ force-pushed the refactor/css-function/parse branch from 9ac8a42 to 8b226e0 Compare June 20, 2024 22:45
@JakeQZ JakeQZ force-pushed the refactor/css-function/parse branch from 8b226e0 to 9329187 Compare June 20, 2024 23:07
@JakeQZ
Copy link
Contributor Author

JakeQZ commented Jun 21, 2024

Looking again, this change is independent of the functional change in #390, so isn't a prerequisite for that implementation.

@oliverklee
Copy link
Contributor

I think I have found a more specific solution to the parse() return type in #608, which I'd like to get in first as a pre-patch.

@JakeQZ JakeQZ force-pushed the refactor/css-function/parse branch 3 times, most recently from c32482c to d80eac3 Compare June 21, 2024 22:58
Also
- Update the `parse` method signature to use PHP7 type hints;
- Remove subsequently redundant DocBlock entries;
- Ditto for same-named method in subclasses to avoid contravariance.

This is the refactoring change from #390.
@JakeQZ JakeQZ force-pushed the refactor/css-function/parse branch from d80eac3 to 875bd71 Compare June 21, 2024 23:03
@JakeQZ
Copy link
Contributor Author

JakeQZ commented Jun 21, 2024

I think I have found a more specific solution to the parse() return type in #608, which I'd like to get in first as a pre-patch.

I've rebased with that change included.

Copy link
Contributor

@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.

LGTM.

@oliverklee oliverklee merged commit ddd7fbf into main Jun 22, 2024
18 checks passed
@oliverklee oliverklee deleted the refactor/css-function/parse branch June 22, 2024 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup refactor For PRs that refactor code without changing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants