Skip to content

Improve rule definitions#706

Open
simonschaufi wants to merge 1 commit into
rectorphp:mainfrom
simonschaufi:improve-rule-definition
Open

Improve rule definitions#706
simonschaufi wants to merge 1 commit into
rectorphp:mainfrom
simonschaufi:improve-rule-definition

Conversation

@simonschaufi

Copy link
Copy Markdown
  • Use active voice
  • Remove final fullstop
  • Use code style

- Use active voice
- Remove final fullstop
- Use code style
public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Change Requires annotations with values to attributes', [
return new RuleDefinition('Change requires annotations with values to attributes', [

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wrong line on phone. I meant this one.

This changes "requires" keyword to word, a @requires might be better

public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition('Refactor deprecated withConsecutive() to willReturnCallback() structure', [
return new RuleDefinition('Refactor deprecated `withConsecutive()` to `willReturnCallback()` structure', [

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you do typos first and leave this function names wrap for next round? I find it hard to review and not sure about this change.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hmm, that will also make it hatte for me and create a lot of extra time to split into another PR. These changes already took me a lot of time and now I need to go through all rules again? I did these kind of changes already before and they went through without causing much trouble in another repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants