-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Enhance Yaml Component readme #17981
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
While I'm 👍 on improve the readme files of all components, I'm 👎 on this because applies to a single component; also we do not use badges in readme files (@fabpot already rejected this a while ago) |
@@ -1,21 +1,33 @@ | |||
Yaml Component | |||
============== | |||
|
|||
YAML implements most of the YAML 1.2 specification. | |||
[](https://packagist.org/packages/symfony/yaml) |
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.
This should be removed, we are not using any badges on symfony/symfony.
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.
As it's a policy on the repo I guess there is not much that can be done here. Still, do you have a means to display the latest composer version available automatically without the badge? Otherwise would a link to packagist be acceptable?
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.
well, you generally don't need to know it as composer require symfony/yaml
will guess a constraint based on the available versions
adding a link to the docs is indeed a good idea. I would revert everything else (the changelog and the license for instance can be easily spotted as the README is displayed along side the main files in the repo). I'm also in favor of removing the "How to run the tests part", which is probably not something people want to do anyway when discovering a new piece of software. And indeed, this should be done on all components. Can you change this one only for YAML and when everyone is ok, you will be able to replicate on all other READMEs. |
and this should probably be done in the 2.3 branch, to apply it to all versions |
|
||
print Yaml::dump($array); | ||
Contributing |
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 would keep such a section, but instead, its content should tell people to use the main repository when contributing (making it less likely to have to move PRs to the main repo, which breaks the development experience as people cannot modify the moved PR after that as it is owned by a core dev)
That would be the case if tests and source files where in folders, but here all the source files are at the root. The more source files there is, the harder it is to spot them.
That's the plan, and will target the 2.3 branch as @stof mentioned. But I wanted to make sure everyone was ok with the change before applying it everywhere. |
|
||
$array = Yaml::parse(file_get_contents(filename)); | ||
[See the Documentation](http://symfony.com/doc/current/components/yaml). |
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.
@fabpot @javiereguiluz should we use https links for the symfony.com website ?
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.
yes
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.
So:
- I need to fix the link
- Wouldn't be possible to get an auto-redirection to HTTPS?
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.
@theofidry symfony.com does not yet enforce HTTPS. but it is best to start using it, as it is available since a few months
Closing in favor of #17997 |
Users, though GitHub, Google or Symfony doc, may be redirected to the Yaml Component GitHub. While the lack of info in the readme is not that big of an issue, as we can guess there is more doc elsewhere and not that many users come there, it can still feel annoying or harmful for others.
This PR aims at proposing simple changes to provide a better experience to people coming on the repo by giving all the info they may require on it:
Also note that this could easily be enforced for other components/bridges as well: it requires minimal change without the risk of breaking any PR.