Skip to content

Add composer runnable scripts for PHPStan & PHP-CS #881

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

Merged
merged 1 commit into from
Jun 20, 2019

Conversation

Steveb-p
Copy link
Contributor

@Steveb-p Steveb-p commented Jun 1, 2019

This allows for forks in composer requiring whole package (enqueue) to
work properly.

Also add code style commands as composer scripts for ease of use.

This allows calling for example composer run cs-fix some-file.php.

@makasim
Copy link
Member

makasim commented Jun 3, 2019

Not sure I get the purpose of this PR, @Steveb-p could you elaborate on why we might need it?

@Steveb-p
Copy link
Contributor Author

Steveb-p commented Jun 3, 2019

@makasim The name for package in composer.json should match the name used in packagist to allow proper forking / repository override.

https://getcomposer.org/doc/05-repositories.md#loading-a-package-from-a-vcs-repository

Use case for this is using your owned version of a library instead of the main one available in packagist - which is helpful when applying some small, custom patches without intent of maintaining a separate package.

I do realize that there should really be no case that whole enqueue-dev repository is used (just sub-repositories instead), but I do see that there are downloads done via packagist.

https://packagist.org/packages/enqueue/enqueue/stats

@makasim
Copy link
Member

makasim commented Jun 3, 2019

tps://packagist.org/packages/enqueue/enqueue/stats is a valid package located here https://github.com/php-enqueue/enqueue-dev/tree/master/pkg/enqueue

The enqueue-dev repo is not even registred on packagist.

@Steveb-p
Copy link
Contributor Author

Steveb-p commented Jun 3, 2019

Then the PR can be discarded.

I can push those scripts declared in composer as a separate PR if you like or drop them entirely - or rename this PR - though I believe they might become helpful for potential contributors - allowing new ones to easily call common tasks without knowing their exact configuration / reading them from travis configuration file. I can also add a note to contributing.md regarding their use.

@makasim
Copy link
Member

makasim commented Jun 7, 2019

I can push those scripts declared in composer as a separate PR if you like or drop them entirely - or rename this PR

That would be great. I am fine with either way.

@Steveb-p Steveb-p force-pushed the match-packagist-name branch from 13fe8ca to bfa6450 Compare June 20, 2019 11:15
@Steveb-p
Copy link
Contributor Author

@makasim I've changed this PR into a one that introduces composer scripts instead only. Figured it would be ok if above discussion is kept instead of creating new PR.

@Steveb-p Steveb-p changed the title Match composer project name with packagist Add composer runnable scripts for PHPStan & PHP-CS Jun 20, 2019
@Steveb-p Steveb-p requested a review from makasim June 20, 2019 11:17
@Steveb-p Steveb-p force-pushed the match-packagist-name branch from bfa6450 to 2fe7b9f Compare June 20, 2019 11:18
@makasim makasim merged commit 0583fc7 into php-enqueue:master Jun 20, 2019
@Steveb-p Steveb-p deleted the match-packagist-name branch October 26, 2019 11:42
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.

2 participants