WPTT / WPThemeReview

PHP_CodeSniffer rules (sniffs) to enforce WordPress theme review coding conventions
MIT License
208 stars 38 forks source link

Add contributing template #176

Closed dingo-d closed 5 years ago

dingo-d commented 5 years ago

Added the contributing template.

dingo-d commented 5 years ago

I've updated the contributing template with the recommended changes and I've also folded the code of the sniff file so that the entire file isn't so long.

I'm waiting on the second round of recommendations :D

dingo-d commented 5 years ago

I've posted on twitter, and on AWP group on facebook, and several times on the themereview channel in slack, and didn't get much feedback.

A colleague of mine suggested that we add a TL;DR version, but I'm not sure how to sumarize this into a short format to be honest.

Can we merge this, or do we want to wait? It would be good to do this as soon as possible because of Twentynineteen theme 🙂

EDIT: Github was scrambled today so I had like 2 comments pasted before, that were the same, sorry if it caused 1000 notifications 😄

joyously commented 5 years ago

@dingo-d Those of us that see your call for feedback don't know enough to give any. We need the contributing document, so can't contribute to it...

jrfnl commented 5 years ago

A colleague of mine suggested that we add a TL;DR version, but I'm not sure how to sumarize this into a short format to be honest.

We could maybe add a table of contents instead ? That way people who go to the document can more easily go to the section which is relevant to them ?

You can use this tool to generate one, though the output does need to be checked & slightly adjusted, but it gives a nice base: https://ecotrust-canada.github.io/markdown-toc/

Can we merge this, or do we want to wait? It would be good to do this as soon as possible because of Twentynineteen theme slightly_smiling_face

I'd say: let's merge this soon. We can always update it later based on feedback. If you like I'll have another read through later tonight to see if I still see anything.

EDIT: Github was scrambled today so I had like 2 comments pasted before, that were the same, sorry if it caused 1000 notifications

No worries, you're good.

N.B.: Oh, one thing though: could you please rebase this PR on the current develop ? And possibly squash the commits into one ? The PR now contains a number of commits unrelated to the change and which have already been merged - see the Commits tab.

dingo-d commented 5 years ago

@joyously The point is to go through the template and see if you can understand how to contribute to the project. Kinda like the first step to contributing 😉

@jrfnl I'll push the TOC and see if it works, and then do the rebasing, but I'll either have to do it late today, or tomorrow morning.

I'm also fine with merging it after this, and then making the changes after that 🙂

dingo-d commented 5 years ago

@jrfnl I think I messed something up when I attempted the rebasing :S Should I try to revert the changes? I really suck at rebasing :/

dingo-d commented 5 years ago

@jrfnl I've made the changes you suggested. Thanks again for fixing all the issues :)

jrfnl commented 5 years ago

@dingo-d Not sure what happened here, but it looks like the rebase we did earlier this week has been undone now.

Want me to re-do it ? (and squash those last commits)

dingo-d commented 5 years ago

Yeah, I guess manually approving the suggested edits undid the rebasing. I think that you can even rebase and squash from the github.

Feel free to do your magic @jrfnl 😄

dingo-d commented 5 years ago

@jrfnl If you'll have the time, can you rebase and merge if everything is ok? Thanks 🙂

jrfnl commented 5 years ago

@dingo-d Looks like it's that last "Reorder" commit which merges the rebased branch back to the old branch....

jrfnl commented 5 years ago

Great job @dingo-d !