-
-
Notifications
You must be signed in to change notification settings - Fork 496
Add current validation performed by the server #36
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
Conversation
README.rst
Outdated
* The versions are valid; | ||
* Version defined in the directory must be the oldest amongst those defined in `version_aliases`; | ||
* The package must exist on Packagist; | ||
* The package must have at least one version on Packagist; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you mean 1 stable version
, 1 released version
(i.e. counting alpha, beta and RC versions) or are you also counting dev branches ?
If dev versions are counted, this is almost impossible to fail this validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, but as I'm making the check anyway, I documented here.
README.rst
Outdated
* JSON files must be valid; | ||
* Aliases are only supported in the main repository, not the contrib one; | ||
* Aliases must not be already defined by another package; | ||
* The versions are valid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"The versions are valid" -> "All versions listed in the manifest exist as package versions on Packagist" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just that they are valid, updating it
Proposal for another check: the GitHub repo has "php", "symfony", "bundle" and "symfony-bundle" topics, as officially recommended (https://symfony.com/blog/standardizing-the-github-topics-for-symfony-repositories). |
Another proposal: XLIFF files must be valid (it's easy to make mistakes and break everything ... I did that recently on the Symfony Demo app: symfony/demo#549) |
Another proposal: service XML files must be valid (we have an XSD for them after all). |
I don't think adding XLIFF files in the recipe makes much sense, so I'm not sure this makes sense to validate them. And if you talk about validating the content of the bundle itself, this would be wrong: it would not forbid you to break your own bundle (as there is nothing triggering a recipe PR in this case), but then your own broken bundle would forbid other people to submit their own recipes (as the recipe of your own bundle, which is already merged, would then become invalid for rules validating content in another repo). |
Another proposal: bundles must be compatible with Symfony. Not simple but the idea is: Symfony Flex server creates an empty Symfony project and then installs the bundle. The check is to run "bin/console --version" and verify that no errors happen. This check is not perfect, but it rules out a lot of incompatibilities. |
@stof yes, I was suggesting to check some of the bundle contents. Using Flex to install a bundle and find that your app is broken is not a good experience (of course it's not caused by Flex ... but if we can use Flex to detect those issues...) |
@javiereguiluz I think those kind of checks are interesting but should be part of a checker that people can install on their bundle repository directly. |
This PR was merged into the master branch. Discussion ---------- Add current validation performed by the server Commits ------- ecc8eb5 added current validation performed by the server
Nothing prevents symfony contributors to create such checker and then integrate it into Flex validation server 😉 |
@Pierstoval it should not be in the flex validation server, as it would have to run when pushing in the bundle repo, not in the flex recipes repo |
No description provided.