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

Add github action for coding style check #677

Merged
merged 7 commits into from
Oct 12, 2023
Merged

Add github action for coding style check #677

merged 7 commits into from
Oct 12, 2023

Conversation

lehecht
Copy link
Contributor

@lehecht lehecht commented Oct 11, 2023

Resolves #676 by adding cs-php action.

Resolves #676 by adding cs-php action
@mzur
Copy link
Member

mzur commented Oct 11, 2023

You have to use the php-cs-fixer branch as base for this PR, otherwise the composer fix command will fail. Also, you can use the php-cs-fixer branch as target for this PR, so this PR will not include all the changes of #652, too.

@mzur mzur mentioned this pull request Oct 11, 2023
@lehecht
Copy link
Contributor Author

lehecht commented Oct 11, 2023

use the php-cs-fixer branch as target for this PR

You mean as new base branch? So php-cs-fixer branch replaces the master branch?

@mzur
Copy link
Member

mzur commented Oct 11, 2023

I meant like this (locally):

  1. You check out the php-cs-fixer branch
  2. You create a new branch from there
  3. You apply the commit (you can git cherry-pick the existing commit from this branch)
  4. You push the branch and create a new PR with the php-cs-fixer branch as target

But it won't matter any more because I'll merge #652 in a few minutes. Then you just have to merge the new master into php-cs-action (or I just do it).

@mzur
Copy link
Member

mzur commented Oct 11, 2023

Now it's working. You can now try to add annotations.

@lehecht
Copy link
Contributor Author

lehecht commented Oct 12, 2023

Cs2pr throws an error, which I cannot reproduce. I'm on it, trying to fix this problem.
If you know something about it, @mzur, please let me know.

@mzur
Copy link
Member

mzur commented Oct 12, 2023

When you use the composer command you have to do it like this:

- composer fix -vvv --dry-run --format=checkstyle | cs2pr
+ composer fix -- -vvv --dry-run --format=checkstyle | cs2pr

Otherwise the arguments will not be forwarded to the CS Fixer command.

composer.json Outdated Show resolved Hide resolved
@lehecht
Copy link
Contributor Author

lehecht commented Oct 12, 2023

Thank you!

Copy link
Member

@mzur mzur left a comment

Choose a reason for hiding this comment

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

It works, nice 👍

You can put adding this action to all the modules on your agenda. When the action is added, all CS issues in the module can be resolved, too. But as I said, you don't have to do this right now so you can work on some more interesting things in the meantime.

@mzur mzur merged commit b0b11b0 into master Oct 12, 2023
12 checks passed
@mzur mzur deleted the php-cs-action branch October 12, 2023 08:19
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.

Add github action to check the code style
2 participants