Skip to content

chore: add .user-guide.php-cs-fixer.dist.php for sample code in docs#5765

Merged
kenjis merged 4 commits intocodeigniter4:developfrom
kenjis:add-.user-guide.php-cs-fixer.dist.php
Mar 9, 2022
Merged

chore: add .user-guide.php-cs-fixer.dist.php for sample code in docs#5765
kenjis merged 4 commits intocodeigniter4:developfrom
kenjis:add-.user-guide.php-cs-fixer.dist.php

Conversation

@kenjis
Copy link
Copy Markdown
Member

@kenjis kenjis commented Mar 3, 2022

Description

  • add .user-guide.php-cs-fixer.dist.php
  • add user guide cs check in git pre-commit hook
  • add user guide cs check in GitHub Action
  • update composer cs-fix

I propose the following before fixing sample code:

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@paulbalandan
Copy link
Copy Markdown
Member

I agree with the proposals. Side comment, latest master of php-cs-fixer has its own implementation of space_after_comment_start so on next release the use on // will be strictly for single-lines only. So, as early as now, multiline comments should start using the /* */ way and reserve // for single-line comments.

@sfadschm
Copy link
Copy Markdown
Contributor

sfadschm commented Mar 3, 2022

I agree with the proposals. Side comment, latest master of php-cs-fixer has its own implementation of space_after_comment_start so on next release the use on // will be strictly for single-lines only. So, as early as now, multiline comments should start using the /* */ way and reserve // for single-line comments.

That sounds reasonable anyway.

@sfadschm
Copy link
Copy Markdown
Contributor

sfadschm commented Mar 3, 2022

Looks good.
This would then be applied and integrated into the pre-commit hook, right?

@kenjis kenjis force-pushed the add-.user-guide.php-cs-fixer.dist.php branch from 7b61054 to 518526b Compare March 4, 2022 00:44
@kenjis
Copy link
Copy Markdown
Member Author

kenjis commented Mar 4, 2022

This would then be applied and integrated into the pre-commit hook, right?

I added the check in the pre-commit.

@kenjis
Copy link
Copy Markdown
Member Author

kenjis commented Mar 4, 2022

I sent PRs #5770, #5771

@sfadschm
Copy link
Copy Markdown
Contributor

sfadschm commented Mar 4, 2022

Applying the fixer will then be another PR?

Because if we merge this now, everyone doing a non-docs PR will also get the style warnings for the docs examples , if I am not mistaken ?

@kenjis kenjis marked this pull request as draft March 4, 2022 07:21
@kenjis
Copy link
Copy Markdown
Member Author

kenjis commented Mar 4, 2022

Applying the fixer will then be another PR?

Yes, because the difference is huge.
But as you say, the code fixes need to be committed soon right after this PR's merging.

@kenjis
Copy link
Copy Markdown
Member Author

kenjis commented Mar 4, 2022

I created #5773 to apply the fixer.

@kenjis kenjis force-pushed the add-.user-guide.php-cs-fixer.dist.php branch from a620e9f to 03354ee Compare March 8, 2022 09:54
@kenjis kenjis marked this pull request as ready for review March 8, 2022 09:55
@kenjis
Copy link
Copy Markdown
Member Author

kenjis commented Mar 8, 2022

Added GitHub Action update.

@kenjis kenjis requested a review from paulbalandan March 8, 2022 10:42
@kenjis kenjis merged commit 9643d44 into codeigniter4:develop Mar 9, 2022
@kenjis kenjis deleted the add-.user-guide.php-cs-fixer.dist.php branch March 9, 2022 04:00
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.

3 participants