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

Try automatic refactoring #527

Closed
elguitar opened this issue May 20, 2021 · 3 comments
Closed

Try automatic refactoring #527

elguitar opened this issue May 20, 2021 · 3 comments
Labels
codebase Refactoring and overall codebase health-related topics.

Comments

@elguitar
Copy link
Contributor

In wp-cli/wp-cli#5528, wp cli project did multiple small refactoring changes in order to remove technical debt.

We should see if the tool used (https://github.com/rectorphp/rector) would be helpful for seravo-plugin.

@elguitar elguitar added the codebase Refactoring and overall codebase health-related topics. label May 20, 2021
@JoosuaKoskinen
Copy link
Contributor

Good idea! I already did some testing with it. There's few sections of code where it gets confused and makes weird proposals, for example:

@@ @@
       <?php elseif ( ! $result ) : ?>
         <p><?php _e('Log empty', 'seravo'); ?></p>
         <?php
+        // result -1 is the signal that something went wrong with reading the log<?php
         // result -1 is the signal that something went wrong with reading the log
         elseif ( $result === -1 ) :
         ?>
@@ @@

But no wonder, even I get confused with that section:

      <?php elseif ( ! $result ) : ?>
        <p><?php _e('Log empty', 'seravo'); ?></p>
        <?php
        // result -1 is the signal that something went wrong with reading the log
        elseif ( $result === -1 ) :
        ?>
        <p><?php _e('Log is broken and can not be displayed.', 'seravo'); ?></p>
      <?php else : ?>
        <p><?php _e('Scroll to load more lines from the log.', 'seravo'); ?></p>
        <?php
      endif;

Definitely not bulletproof tool but with correct configuration it could be very helpful. So far it has made a lot of smart decisions.

@JoosuaKoskinen
Copy link
Contributor

JoosuaKoskinen commented May 20, 2021

Made an upstream issue to see what they think: rectorphp/rector#6443. Not a big issue but doesn't increase my trust in Rector and the target was to make the codebase cleaner.

@Moppa5
Copy link
Contributor

Moppa5 commented Jun 11, 2021

Closing this issue as it seems that Rector is here to stay and the new development tools have been merged to master. We can discuss Rector specific issues or improvements separately.

@Moppa5 Moppa5 closed this as completed Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codebase Refactoring and overall codebase health-related topics.
Projects
None yet
Development

No branches or pull requests

3 participants