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 Actions config for syntax checking #895

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

samwilson
Copy link
Contributor

@samwilson samwilson commented Jul 13, 2018

This adds continuous-integration configuration for checking PHP syntax.

Refs: #817

@samwilson samwilson force-pushed the travis branch 2 times, most recently from 6abc4c6 to 79cda36 Compare July 13, 2018 01:28
@samwilson
Copy link
Contributor Author

Lots of these tests fail at the moment :-) I temporarily added my fork to Travis https://travis-ci.org/samwilson/Piwigo and you can see an example of the results at https://travis-ci.org/samwilson/Piwigo/jobs/403372472

@flop25
Copy link
Member

flop25 commented Jul 13, 2018

99.9% of 'Line indented incorrectly'
thanks for your contributions

@samwilson
Copy link
Contributor Author

I'm not sure if you meant it, but that comes across as sarcastic. I'm actually trying to help here!

I'm not suggesting that the phpcs ruleset is complete or correct, and of course it needs to be fixed up. The idea here is that automatic checks be done. The details of those checks need to be figured out. :-)

@flop25
Copy link
Member

flop25 commented Jul 13, 2018

I'm not suggesting anything neither

@samwilson
Copy link
Contributor Author

samwilson commented Jul 13, 2018

I'm not suggesting anything neither

Okay, sorry, I took your comment wrongly! :-) Is easily done in these text worlds.

Anyway, I've removed the indenting rules. There are still lots of errors, but they can all (I think) be fixed automatically.

There are lots of files in language/ with their executable bits set.

@samwilson samwilson force-pushed the travis branch 3 times, most recently from bfa4955 to f22c11b Compare July 14, 2018 00:39
@samwilson samwilson changed the title Add Travis config for various checks Add Travis config for syntax checking Jul 14, 2018
@samwilson
Copy link
Contributor Author

I've remove all but the syntax checking from this PR, to keep it simpler. The build is now passing:
https://travis-ci.org/samwilson/Piwigo/builds/403788915

The changes to xmlrpc_encoder.php are to wrap the xmlrpc_encode() function in a conditional, but I didn't re-indent the actual function just to keep the diff cleaner.

@samwilson samwilson mentioned this pull request Jul 14, 2018
@plegall
Copy link
Member

plegall commented Jul 14, 2018

Adding Continuous Integration with automated check is great. I'll review it next week. Thank you @samwilson

@samwilson
Copy link
Contributor Author

I've brought this up to date, in case it's still of interest. See example results at https://travis-ci.org/samwilson/Piwigo/builds/571741891

@samwilson
Copy link
Contributor Author

I've brought this up to date and switched it to use Github Actions. Results are at https://github.com/samwilson/Piwigo/actions/runs/127365045

@samwilson samwilson changed the title Add Travis config for syntax checking Add Github Actions config for syntax checking Jun 7, 2020
@plegall
Copy link
Member

plegall commented Jul 23, 2022

Hi @samwilson

I'm reviewing this pull-request. I don't know yet how to deal with it. I think it would be interesting indeed to have an automatic check on syntax and maybe on coding style (which would already be complex to configure I guess).

This adds continuous-integration configuration
for checking PHP syntax.

Refs: Piwigo#817
@samwilson
Copy link
Contributor Author

I've added PHP 8.0 and 8.1 to this patch now (and removed the comment about only testing against supported PHP versions, because it's good to keep testing against all working versions). You can see the current results at https://github.com/samwilson/Piwigo/actions/runs/2729396577

I think having at least a basic syntax-check would be a good thing (to avoid issues such as #894), and as you say it could eventually be extended to also include coding style (I started some work on a phpcs standard at https://github.com/samwilson/piwigo-coding-standards ).

Let me know if there's more info that should be added to this patch.

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