Add contributing guidelines

MyIntervals / PHP-CSS-Parser

A Parser for CSS Files written in PHP. Allows extraction of CSS files into a data structure, manipulation of said structure and output as (optimized) CSS

http://www.sabberworm.com/blog/2010/6/10/php-css-parser

MIT License

1.76k stars 148 forks source link

Add contributing guidelines #489

Open JakeQZ opened 8 months ago

JakeQZ commented 8 months ago

Relates to #486.

[ ] general workflow
[ ] mention starter issues
[ ] about code reviews
[ ] installing development dependencies + PHIVE
[ ] unit-test your changes
[ ] creating PRs
[ ] rebasing
[ ] write a few sentences on git commit messages and link https://cbea.ms/git-commit/
[ ] write something on test naming and link https://speakerdeck.com/oliverklee/test-driven-development-with-phpunit-915a5f2c-3d37-4e41-b5bc-ef1efca9c01e?slide=39
[ ] mention that the changelog is in reverse chronological order (newest on top)
[ ] move the developer instructions at the end of the README there, and replaced them with a link
[ ] In general, please always use the third person for the first sentence of a method documentation comment: "(This method) Does …"; example: https://www.php.net/manual/en/function.md5-file.php
[ ] mention that this project uses Hungarian notation and that new code should not use it anymore (as we'll stop doing that)
[ ] mention the coding style
[ ] mention the developer tools
[ ] document how to create PRs for mass removals/mass deprecations: https://github.com/MyIntervals/PHP-CSS-Parser/issues/512#issuecomment-2178201337

oliverklee commented 4 months ago

Also link https://cbea.ms/git-commit/ for git commit messages and https://speakerdeck.com/oliverklee/test-driven-development-with-phpunit-915a5f2c-3d37-4e41-b5bc-ef1efca9c01e?slide=39 for test naming.

oliverklee commented 4 months ago

Also mention that the changelog is in reverse chronological order (newest on top).

ziegenberg commented 4 months ago

Also link https://cbea.ms/git-commit/ for git commit messages and https://speakerdeck.com/oliverklee/test-driven-development-with-phpunit-915a5f2c-3d37-4e41-b5bc-ef1efca9c01e?slide=39 for test naming.

I'm assuming that is only additional. As someone on the consuming side of contributor guidelines, I appreciate having those things readily available in a document within the project. That way, it is directly accessible to me using the same tool I'm currently working in, and I can search for it. I wouldn't say I like having to click several links, read text from images or dig through a lengthy blog post and contribute to someone else's statistical data by visiting 3rd party websites.

ziegenberg commented 4 months ago

Other things to mention in the guidelines:

In general, please always use the third person for the first sentence of a method documentation comment: "(This method) Does …":

ziegenberg commented 4 months ago

Probably also add, that this project uses Hungarian notation.

oliverklee commented 4 months ago

Probably also add, that this project uses Hungarian notation.

We'd actually like to get rid of that as part of the 9.0 milestone:

rename internal things (including parameter names) to ditch the Hungarian notation

JakeQZ commented 4 months ago

Also (though this is probably more for maintainers and collaborators), for PRs that (possibly indirectly) change coding style and guidance, or the set of developer tools and how they are installed, update the CONTRIBUTING.md file. The file referenced in the OP has become out of date since this has not been done (https://github.com/MyIntervals/emogrifier/issues/373).

JakeQZ commented 4 months ago

Other things to mention in the guidelines:

In general, please always use the third person for the first sentence of a method documentation comment: "(This method) Does …":

Examples would help for this one. "(This method) ..." may be confusing, and lead people to think they should explicitly type out "This method", then wonder about the brackets.

This PHP documentation is inconsistent, often using the imperative or infinitive (unclear which, because it's English) in the summary if not the description too. After a bit of trawling, I found md5_file as a reasonable example.

ziegenberg commented 4 months ago

Other things to mention in the guidelines:

In general, please always use the third person for the first sentence of a method documentation comment: "(This method) Does …":

Examples would help for this one. "(This method) ..." may be confusing, and lead people to think they should explicitly type out "This method", then wonder about the brackets.

This PHP documentation is inconsistent, often using the imperative or infinitive (unclear which, because it's English) in the summary if not the description too. After a bit of trawling, I found md5_file as a reasonable example.

This suggestion was originally posted by @oliverklee in https://github.com/MyIntervals/PHP-CSS-Parser/pull/545#discussion_r1644011198. I referenced it here as I felt this could be a general guideline, which in this case should probably become part of CONTRIBUTING.md.

oliverklee commented 4 months ago

Also document how to create PRs for mass removals/mass deprecations: https://github.com/MyIntervals/PHP-CSS-Parser/issues/512#issuecomment-2178201337

JakeQZ commented 4 months ago

Examples would help for this one. This suggestion was originally posted by @oliverklee in #545 (comment). I referenced it here as I felt this could be a general guideline, which in this case should probably become part of CONTRIBUTING.md.

Kudos. Just suggestiong how it should become part of CONTRIBUTING.md. There isn't one yet on this project. Someone needs to kick the ball rolling...