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

GitHub workflow adjustments #2969

Merged
merged 2 commits into from Feb 22, 2020
Merged

GitHub workflow adjustments #2969

merged 2 commits into from Feb 22, 2020

Conversation

micgro42
Copy link
Collaborator

@micgro42 micgro42 commented Jan 21, 2020

This should enable plugins to natively run PHP Code Sniffer checks against the actual DokuWiki standard and not against a version that has been watered down until we have gotten around to fixing all the violations.

An example for such a workflow can be found with the struct section plugin: .github/workflows/phpCS.yml.

(Notice that this file is suitable to be included in the plugin-wizard)


PS: It is currently not reasonably possible to add namespaces to the classes like action_plugin_structsection. I wonder if we could at some point refactor the plugin system to support this?

This should make the code style checks also run on pull requests the
come from forks. Maybe also on old pull requests?
@micgro42 micgro42 force-pushed the workflowAdjustments branch 2 times, most recently from 9472f2c to f16368f Compare January 21, 2020 22:32
@micgro42 micgro42 marked this pull request as ready for review January 21, 2020 22:36
@splitbrain
Copy link
Collaborator

Looks good to me, though personally I would like to keep allowing Generic.ControlStructures.InlineControlStructure.NotAllowed because I quite like to have simple one-line guardians at the top of a function. AFAIK PSR-2 does not forbid these, not sure about PSR-12.

@micgro42
Copy link
Collaborator Author

Mh, PSR-2 does say:

  1. Control Structures

The general style rules for control structures are as follows:

[...]
The closing brace MUST be on the next line after the body

The body of each structure MUST be enclosed by braces. This standardizes how the structures look, and reduces the likelihood of introducing errors as new lines get added to the body.

PSR-12 is even a bit more verbose.

That being said, we are free to diverge from PSR as we like. We are creating our own code style after all :)

Splitting them out allows for plugins to use _test/phpcs.xml as the
basis for their own linting without having an overly permissive coding
standard.
Also, this makes it more obvious and painful that these are just
intended as temporary exceptions and should be actually fixed.

The rule `Generic.ControlStructures.InlineControlStructure.NotAllowed`
has its comment adjust to make it clear that this is an intended
deviation from the PSR-2/PSR-12 coding standard.

The rule `PSR1.Classes.ClassDeclaration.MissingNamespace` has to remain
in the DokuWiki coding standard as the plugin base classes can currently
not reasonably be in namespaces.
Copy link
Collaborator

@phy25 phy25 left a comment

Choose a reason for hiding this comment

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

LGTM and I triggered a retry on travis, which turns green now.

I love the idea of splitting the temporary rules to a new file.

@splitbrain splitbrain merged commit 6cb645b into master Feb 22, 2020
@splitbrain splitbrain deleted the workflowAdjustments branch February 22, 2020 20:27
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