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

Run static code analysis off GitHub Actions #5295

Merged
merged 1 commit into from Nov 26, 2020
Merged

Conversation

sanmai
Copy link
Contributor

@sanmai sanmai commented Nov 23, 2020

.github/workflows/sca.yaml Outdated Show resolved Hide resolved
@sanmai
Copy link
Contributor Author

sanmai commented Nov 24, 2020 via email

@sanmai
Copy link
Contributor Author

sanmai commented Nov 24, 2020

git diff --name-only --diff-filter=ACMRTUXB HEAD~..HEAD | grep -E "\.php$" || true
echo 'CHANGED_PHP_FILES<<EOF' >> $GITHUB_ENV
git diff --name-only --diff-filter=ACMRTUXB HEAD~..HEAD | grep -E "\.php$" || true >> $GITHUB_ENV
echo 'EOF' >> $GITHUB_ENV
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

i just hope this section works

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part is from Travis CI, and the other part I took from Infection CI.

Even if these don't work for some unknown reason, we should have plenty of the debugging output. I'll be here to fix any issues.

command: |
./dev-tools/install.sh

- name: Run checks
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just the noise from GH actions wrapper. You can see it at every step.

I think they're trying to tell us which shell we're using, and with which arguments.

- name: Run checks
run: |
./dev-tools/check_file_permissions.sh
./dev-tools/check_trailing_spaces.sh
Copy link
Member

Choose a reason for hiding this comment

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

question: will check_trailing_spaces executes if check_file_permissions would report issue?
(we don't want to execute in such case, and with Travis we had to always kill the flow manually with travis_terminate)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will stop right then right here because the shell runs with set -e. Note the -e argument to bash above.

keradus added a commit that referenced this pull request Nov 26, 2020
This PR was merged into the 2.16 branch.

Discussion
----------

Add yamllint workflow, validates .yaml files

This has two effects:

- It will validate `.yaml` files for general correctness everywhere except `vendor` and `dev-tools/vendor`
- And as a side-effect, it might enable other workflows to happen. E.g. we will no longer need to consult with forks to see if  #5295 #5268 #5183 etc are working.

Commits
-------

f6cdee1 Add yamllint workflow
Add changed files check, disable the same checks from Travis CI

Remove conflict with HHVM
@sanmai
Copy link
Contributor Author

sanmai commented Nov 26, 2020

Rebased. Should be showing the status right here.

@keradus keradus added this to the 2.16.8 milestone Nov 26, 2020
@keradus
Copy link
Member

keradus commented Nov 26, 2020

Thank you @sanmai.

@keradus keradus merged commit 66c81e0 into PHP-CS-Fixer:2.16 Nov 26, 2020
@sanmai sanmai deleted the GhSCA branch November 26, 2020 22:52
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.

None yet

3 participants