Skip to content
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

IBX-8117: Prepared Rector package #1

Merged
merged 12 commits into from
May 10, 2024
Merged

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented May 8, 2024

🎫 Issue IBX-8117

Description:

This PR provides some skeleton which adapts https://github.com/rectorphp/rector to our needs.

Changes:

  • required rector/rector v1
  • forced nikic/php-parser ^4 due to php-parser v5 compat rectorphp/rector#8567.
  • dropped integration tests setup as they seemed not to apply to this package needs (there's Rector-given test framework)
  • added placeholder for Ibexa DXP v5.0 ruleset
  • documented installation, usage, and writing custom rules guide
  • bumped PHPUnit to ^10 to be able to use Rector-provided PHPUnit template for testing custom rules
  • added test coverage for essential parts
  • defined a Composer-wrapped Symfony command define-custom-rule and necessary processors, based on built-in Rector custom-rule command, utilizing rector-provided templates

Background: built-in command is not very flexible and does too much, adding our custom command based on the built-in seemed like the most sane approach (though required not small portion of code). Road explored: wrap it and move files afterwards, but that doesn't provide the best DX as the command messes with composer and PHPUnit configuration...

Please note that this is executed by composer define-custom-rule so there's no Symfony DI involved there, nor bundle requirement. It makes it more compact and easier to install and use, though it's something that can be changed, depending on the preferences.

For QA:

Rather no QA at this point.

Documentation:

All needed information were documented here. Due to the nature of this package IMHO it makes more sense to do it here, however we can still either reference this in the Official Doc or move it and reference Official Doc here.

@alongosz alongosz force-pushed the ibx-8117-prepare-rector-package branch from 2fb6ad2 to 121bf48 Compare May 9, 2024 08:42
@alongosz alongosz requested review from a team and mnocon May 9, 2024 08:46
Copy link

@mnocon mnocon left a comment

Choose a reason for hiding this comment

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

Looks good to me 💪

README.md Show resolved Hide resolved
phpunit.xml.dist Outdated Show resolved Hide resolved
alongosz and others added 2 commits May 10, 2024 11:03
Co-Authored-By: Paweł Niedzielski <Steveb-p@users.noreply.github.com>
Co-Authored-By: Paweł Niedzielski <Steveb-p@users.noreply.github.com>
@alongosz alongosz requested a review from Steveb-p May 10, 2024 09:13

## Creating custom rules

This package comes with a Composer-wrapped Symfony command to generate custom rule and place its files to proper
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This package comes with a Composer-wrapped Symfony command to generate custom rule and place its files to proper
This package comes with a Composer-wrapped Symfony command to generate custom rule and place its files in proper

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, missed this. Fixed via ee2b174 on main, thanks.

@konradoboza konradoboza requested a review from a team May 10, 2024 09:26
@alongosz alongosz merged commit 303e0c6 into main May 10, 2024
3 checks passed
@alongosz alongosz deleted the ibx-8117-prepare-rector-package branch May 10, 2024 09:28
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.

5 participants