-
Notifications
You must be signed in to change notification settings - Fork 74
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
Rector 0.10 with minimal changes. #144
Conversation
# Conflicts: # .editorconfig # composer.json # config/drupal-8/drupal-8.0-deprecations.yml
Also adds PHPStan workflow to catch unused calls.
Trying to see if this may work with just the rector changes.
Remove ones missed in the last commit and update the functional test.
/drupal-rector /web
Not sure of the status of this. I see it was cut back, which I generally agree makes reviewing easier. There was lots of testing of the other PR though at DrupalCon, etc. Do we consider that covered this or does this need more eyes? |
I think it would be good to get a few additional people to run the tool locally to see if it works as expected. It sounds like people were able to provide feedback from the end user perspective. I'll add a few remaining tasks to the description and I can probably take a look next week. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@damontgomery - I am a dev and a user: I'm a Drupal module maintainer. I successfully used Drupal rector last year to help get our code base from D8->D9 and I was able to easily install @mglaman 's #134 PR during DrupalCon -> and with that PR I was able to successfully run rector on a number of Drupal module projects - including a sizeable one like Webform module and our own Webform CiviCRM module. And yes I did this on a Drupal 8 site (same infra/dev I had previously used Matt's PR on). I really just tried testing this PR b/c @goba asked me to do so. Sorry I couldn't get it working. |
Thanks again for the feedback, @KarinG. |
👍 Testing InstructionsOpen a Drupal project (I used DDEV + Gitpod) Important: Make sure it's Drupal 8 (and NOT Drupal 9)
Configure it to use latest drupal-rector
Require the specific branch for this PR
Configure it
Grab the example
Or test with your own modules
|
Thanks for testing @shaal and for the testing notes! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursory overview, it contains the right stuff 👍
I have created a Drupal.org issue to cover this PR, and recommend that we merge it as a 0.10.x branch, pinned to main here. |
This is the work in #134, but without the developer tool changes.
Remaining work
If people want to merge this in as a beta release while we work on documentation / support, I'm open to suggestions. See below for a bit more questions around that.
.yml
to.php
for the configuration fileRelease process request for feedback
Does this make sense as a beta release?
With the changes to the end user and developer experiences, should we coordinate a blog post / video about changes?
Testing notes
Requirements
Drupal Rector needs to be installed as a package on a Drupal 8 site. If you would like to use the
drupal-rector-sandbox
project, this may make testing easier since it includes a step to link the examples into the modules folder and creates adrupal-rector
folder inside the repository where you can use Git to check out branches. If you want to use an existing Drupal 8 project to test this, you will need to updaterepositories
incomposer.json
and make sure you download this branch.Set up a local testing environment
drupal-rector-sandbox
project, https://github.com/palantirnet/drupal-rector-sandboxdrupal-rector-sandbox/drupal-rector
folder.drupal-rector-sandbox
,cd drupal-rector
git fetch --all
git checkout origin/rector-0.10-minimal-changes
drupal-rector-sandbox
composer update palantirnet/drupal-rector -W
.php
configuration file into the sandboxrm rector.yml
from thedrupal-rector-sandbox
foldercp drupal-rector/rector.php .
from thedrupal-rector-sandbox
folderEnd user testing notes
vendor/bin/rector process --dry-run web/modules/custom/rector_examples/db_select.php
or something similar fromdrupal-rector-sandbox
.Developer testing notes
composer install
from thedrupal-rector
foldervendor/bin/behat
from thedrupal-rector
folderAttributions
This is all Matt Glaman's work that was built on the work by Tomas Votruba.
Matt also included a lot of great automated testing / developer tools. Unfortunately, I was not able to get them to work myself, so I wanted to include the work I could verify and keep the (probably temporary) functional Behat tests so that I could test locally / on Github Actions.
If anyone would like to help with the update to the tools that Matt worked on including testing and documentation, it would be greatly appreciated.