-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
lint:yaml command as a separated package #18987
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
Comments
Is |
If it show deprecations it could be worth it though… for instance, not quoting service ids has been deprecated lately. |
The command |
yeah, and this is only about supporting things like |
The last question is : in which component this generic command should be placed ? Currently, it depends on IMO, the Yaml component is the best candidate to host this command because that's a feature of Yaml. |
Agree. And what about having a binary? |
I would vote for the Yaml component as well. |
Is it convenient to introduce a class in the Yaml component that depends on the Console+Finder ones? I personally made a private port of the YamlLintcommand as a single-command app ( |
the finder dependency could be avoided actually, by using SPL iterators directly without the nice API of the Finder (should be easy as the command just does a recursive iteration and then filters files by extension. It does not rely on fancy features of the Finder). |
In fact a standalone library providing only a linter sounds overkill. |
…ml component (chalasr) This PR was merged into the 3.2-dev branch. Discussion ---------- [FrameworkBundle][Yaml] Move YamlLintCommand to the Yaml component | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #18987 | License | MIT | Doc PR | ~ See #18987 for the use case. Also I see several things that can be simplified/optimized in the original command, but I think it would be better to propose the changes in a next PR and just propose the exact changes needed to move it outside of the framework. If all should be done here, let me know before reviewing it. Commits ------- 33402ea Add Yaml LintCommand
@chalasr It's a good start, but in this case we still have to create the console file calling this command. I still think having a ready-to-use package is a good idea. WDYT it's overkill? |
@soullivaneuh I quickly changed my mind on that. But I still think you're right about that having it as a package makes sense. It looks like there is a similar intent for twig btw: twigphp/twig-lint |
I agree with @soullivaneuh's proposal. I'd love something simple that I can use in Travis CI and deployment scripts. Thanks! |
I was looking into creating a small package for this. But in fact there already is one: https://github.com/certificationy/yaml-linter |
@xabbuh I wasn't asking for a separate package/component/bundle/etc. I just need that the component itself provides a ready-to-use linter binary. Something like this:
The "binary" would be a minimal Symfony console app with just 1 command. |
@javiereguiluz I prefer the |
👍 for putting such binaries into the |
We now provide binaries in components (e.g. #26654), reopening. |
Hello @chalasr, is something like that useful? https://github.com/certificationy/yaml-linter How can I help with that? Mickaël |
Another one: https://github.com/asm89/twig-lint |
What would be the goal of a separate package? When I recently made an attempt to improve the linting capabilities of the YAML component (#30137), it was rejected because there are already better linters out there. |
To me, we only need to provide an executable in the Yaml component. Syntax validation is part of the Yaml component features, there is no point in making it a dedicated package as soon as the component provides the code needed to use it. |
Contributing the lint.php file directly into the Yaml component is enough to fix this issue or do you need more? |
Yes, nothing more to me. PR welcomed from anyone, could be nice to let @igor-susic doing it for its first contribution. |
@chalasr If others agree, I would like to be assigned and deal with this one 😄 |
@igor-susic Are you still working on this one? I would be happy to provide a PR in case you don't have time. |
Another one https://github.com/HeahDude/yaml-linter |
This PR was merged into the 5.1-dev branch. Discussion ---------- [Yaml] Added yaml-lint binary | Q | A | ------------- | --- | Branch? | 5.1 | Bug fix? | no | New feature? | yes | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #18987 <!-- prefix each issue number with "Fix #", if any --> | License | MIT | Doc PR | tbd. <!-- Replace this notice by a short README for your feature/bugfix. This will help people understand your PR and can be used as a start for the documentation. Additionally (see https://symfony.com/roadmap): - Always add tests and ensure they pass. - Never break backward compatibility (see https://symfony.com/bc). - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too.) - Features and deprecations must be submitted against branch 4.4. - Legacy code removals go to the master branch. --> Commits ------- 2640dfe [Yaml] Introduce yaml-lint binary
Currently, if we want to use the
lint:yaml
command, we can only do this on a Symfony Full Stack installation, or require FrameworkBundle and add the command under a self made application file.It would be great if we can have this one as a binary, like php-cs-fixer for example.
This would be useful to integrate it under Travis for example, for our bundles.
An equivalent already exists for Ruby: https://github.com/Pryz/yaml-lint
Or maybe move it under the yaml component?
What do you think?
The text was updated successfully, but these errors were encountered: