Skip to content

Comments

Only set installed_paths if there's no default#123

Open
rmccue wants to merge 3 commits intomainfrom
use-phpcs-conf
Open

Only set installed_paths if there's no default#123
rmccue wants to merge 3 commits intomainfrom
use-phpcs-conf

Conversation

@rmccue
Copy link
Member

@rmccue rmccue commented Mar 10, 2020

Obsoletes #108. Fixes #97.

This checks for the presence of phpcs' internal configuration file (i.e. the one use for --config-set/etc) at vendor/squizlabs/php_codesniffer/CodeSniffer.conf. When dealerdirect/phpcodesniffer-composer-installer is required, it runs phpcs --config-set installed_paths ... after any package is added or removed. This creates the config file.

Installed paths are now only set manually if there's no configuration, allowing us to bundle this properly into the standard instead.

@rmccue
Copy link
Member Author

rmccue commented Mar 10, 2020

Tested with 0.4.2 (which doesn't have the conf file, so defaults are set correctly) and latest (which does, and uses that instead) and all works well. 👍

@rmccue rmccue requested a review from mikeselander March 10, 2020 13:34
]

// Only include the VIP WPCS if the path exists within this version of the standards.
if ( fs.existsSync( path.join( standardPath, 'vendor', 'automattic', 'vipwpcs' ) ) ) {
Copy link
Contributor

@mikeselander mikeselander Mar 10, 2020

Choose a reason for hiding this comment

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

Seems like we can just add this to the above list if we're using DealerDirect, ya?

I managed to totally misread this 😬ignore me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we even need this if we're far back enough to not have had DealerDirect?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think so? I think we had overlap.

]

// Only include the VIP WPCS if the path exists within this version of the standards.
if ( fs.existsSync( path.join( standardPath, 'vendor', 'automattic', 'vipwpcs' ) ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we even need this if we're far back enough to not have had DealerDirect?

@rmccue rmccue requested a review from mikeselander June 10, 2020 13:08
@rmccue
Copy link
Member Author

rmccue commented Jun 10, 2020

Ah nope, this still needs updating. My bad.

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.

Re-consider PHPCS installed_paths argument

2 participants