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

Make an invalid package name a publish error, not warning #4118

Merged
merged 1 commit into from Jun 18, 2015

Conversation

legoktm
Copy link
Contributor

@legoktm legoktm commented Jun 8, 2015

Since invalid names will prevent publishing to packagist, they should
be considered publish errors. If people do not plan on submitting their
package to packagist, they can use the --no-check-publish flag to turn
it into a normal warning again.

Since invalid names will prevent publishing to packagist, they should
be considered publish errors. If people do not plan on submitting their
package to packagist, they can use the --no-check-publish flag to turn
it into a normal warning again.
@alcohol
Copy link
Member

alcohol commented Jun 8, 2015

Packagist already fails on warnings, so there is no need for this. The warning also explicitly states they will not be able to publish to packagist.

The $publishErrors array only seems to be used when parsing the composer.json (which either end up in $publishErrors or just $errors). Not sure if it should contain warnings such as these.

@legoktm
Copy link
Contributor Author

legoktm commented Jun 8, 2015

My usecase here is to have CI fail when someone tries creating a package with an uppercase name in it. We currently use composer validate to do that. But since it's just a warning, it doesn't return a failure exit code, causing CI to pass.

This is kind of a follow up to #3804, which officially differentiated between validation errors and publish errors, and let people ignore publish ones.

@alcohol
Copy link
Member

alcohol commented Jun 9, 2015

Upon further review I agree, in the context of the validate command this should be a publish error. See also ValidateCommand.php#L82-L86.

@Seldaek Seldaek merged commit 49bd1d7 into composer:master Jun 18, 2015
@Seldaek
Copy link
Member

Seldaek commented Jun 18, 2015

Makes sense yes thanks

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