Use Rector to autofix code smell #4045
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A workflow should automatically suggest a new pull request with the changes. Initial fixes will not be applied from the auto generated PR but will be added as piece by piece commits with rule updates to this branch.
when using bootstrapping, these defines seem not to be auto detected anymore. We probably have to add a few more once we start running on plugins.
Seems rector was too aggressive here in removing a variable.
This broke tests. Ideally an Interface should be defined that both the real APICore and the TestPlugin implement.
prevent rector from rewriting this into an auto-assignment.
Klap-in
reviewed
Aug 29, 2023
|
Good news: all the code sniffer rules are now applied as well. |
|
Please double check my usage of $auth instanceof AuthPlugin that I did not swap the logic. |
mprins
reviewed
Sep 1, 2023
fiwswe
reviewed
Sep 1, 2023
I'm not 100% sure but revs might be strings sometimes, so better not chage current behavior
current revision is often passed as empty string
|
Okay, I think I'm through and addressed all comments. @Klap-in your auth instance changes look correct to me. Unless someone objects, I would merge this later today. We can (and should) always do small improvements later. |
Klap-in
approved these changes
Sep 2, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a configuration for rector and a new github workflow to automatically apply it when new code is pushed to master. The code is then also autofixed by php codesniffer. The result is auto submitted as pull request.
This PR also applies the changes suggested by rector in batches (see commits). I looked at all the changes made to ensure they seem valid, removed offending rules where it didn't and also manually reformatted some of the output. The latter is needed because rector isn't very good with white spaces and array formatting.
Ideally this is merged quickly to avoid merge conflicts but also to get the changed code into other people's hands to see if anything broke that was not found by tests.
The ultimate goal is to provide this mechanism to plugin authors to automatically update their plugins to new DokuWiki and PHP standards.
Post merge: