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

API Apply psr-2 coding conventions to framework with linting CI check #6340

Merged
merged 3 commits into from
Nov 29, 2016

Conversation

tractorcow
Copy link
Contributor

Initial commit of #6096

Note that all non-automatic fixes have been excluded from this initial build; Manual resolution of all outstanding linting issues are outside of the scope of this introductory pull request.

As requested by @sminnee I've split into two commits, one is an automatic cleanup and the other a linting check.

I have also excluded tests from the current linting, but the plan is to eventually fix and add this folder too.

PHPCS_TEST is run in PHP7 build to ensure optimal coverage of all language constructs.

@tractorcow
Copy link
Contributor Author

PHPCS_TEST is green. :) At least, once I muted all the known broken rules :P

@sminnee
Copy link
Member

sminnee commented Nov 28, 2016

My preference would have been for the whitespace changes to have been applied in their own commit, but it's not a merge blocker if that would be more than an hour's rework.

@tractorcow
Copy link
Contributor Author

All of the automatic fixes are essentially whitespace-only changes. I ran the upgrade again last night with your suggested commands and there was very little difference in output.

I don't think phpcbf can easily fix non-whitespace changes without manual input. I may need to double check though.

@sminnee
Copy link
Member

sminnee commented Nov 28, 2016

The other thing I suggest is setting up a custom script in composer.json so that you can lint by running composer run-script lint.

This will make it easier to lint locally without having to copy/paste a command out of .travis.yml.

@tractorcow
Copy link
Contributor Author

Done, and CI uses the composer script to run linting.

Copy link
Member

@sminnee sminnee left a comment

Choose a reason for hiding this comment

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

Looks good; let's follow this up with a tests PR.

@sminnee sminnee merged commit d38097e into silverstripe:master Nov 29, 2016
@tractorcow tractorcow deleted the pulls/4.0/psr-2 branch November 29, 2016 05:07
chillu added a commit to open-sausages/silverstripe-framework that referenced this pull request Dec 8, 2016
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

2 participants