Skip to content
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

Add contribution guidelines #428

Merged
merged 33 commits into from Aug 11, 2019
Merged

Conversation

bahiirwa
Copy link
Collaborator

Change in docs to make guided contributions in PRs
Reference: ClassicPress/ClassicPress-v1#144

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@nylen nylen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the direction this is going, but it is not ready for inclusion yet. The new content needs to be added to a new CONTRIBUTING.md document, which is the GitHub standard for contribution instructions.

The contributing document should also be structured as a start-to-finish guide to submitting an issue or a change to ClassicPress, including a note about the petitions process for major changes, and a note to submit an issue instead of a PR if someone isn't sure how their proposed change fits within the guidelines.

We can then link to this document from the readme and from the issue/PR templates.

Cleanup readme to make a contributions.md file
Add automated unit tests link
Initial commit for contributing.md
Add quick link to help notes in contributing.md
@bahiirwa
Copy link
Collaborator Author

I have made a PR refresh. Some parts of the original message in backporting WP changesets were vague to me. Please include any changes that are missing for the process to be better.

File name change and inclusion of a code of conduct. I realized we have something very limited on the CP website about conduct and this is only in the forums.
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Keep the code of conduct cutting across the platforms.
Change casing to the right CONTRIBUTING.md file
Copy link
Contributor

@Mte90 Mte90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job!
I was already following the pr but now is in a very good status, just few changes to improve it the readability for newcomers to the community.

.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
@bahiirwa
Copy link
Collaborator Author

Please review this PR and kindly let me know what needs to be pushed for it to be completed.

Specific action items, with brief explanation of how our process works
along with the appropriate links.
Try to explain why (in what situations) a contributor would use each of
these channels.
Explain the how and why of backporting changes from WordPress.
@nylen nylen changed the title Update docs on bugs and changesets Add contribution guidelines Aug 1, 2019
@nylen
Copy link
Contributor

nylen commented Aug 1, 2019

I've pushed some suggested updates, trying to make the document flow better with clearer explanations of each part. Please let me know what you think, and I'll also leave a few specific comments.

- [Slack](https://www.classicpress.net/join-slack/) - great for real-time chat, posting screenshots and asking questions about anything you're stuck on. Or just socializing.
- [Petitions website](https://petitions.classicpress.net/) - for proposing new features or major changes for consideration by the community.
- [GitHub issues](https://github.com/ClassicPress/ClassicPress/issues) - for proposing or discussing bugfixes, or minor improvements. Generally it is a good idea to create a petition for anything that may take a significant amount of time or that may have backwards compatibility implications.
- [Forums](https://forums.classicpress.net/) - a lot of good info and discussion here too
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our forums are a great resource, I want to say something better than "a lot of good info and discussion here too". In what situations would someone want to make a forum thread rather than using one of the other communication channels?


If you're not sure where to start contributing, here are some ideas:

- Review and test [existing PRs](https://github.com/ClassicPress/ClassicPress/pulls), especially looking at how well they fit the criteria described above.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this sentence does a good job of stating how important reviewing and testing are. We need to be thorough and careful with the changes we make, and the more eyes we can get on our PRs the better.

@@ -9,6 +9,8 @@ https://petitions.classicpress.net/

For more details about our governance and change planning process, see:
https://www.classicpress.net/democracy/

Please find guidelines on how to contribute to this project in our [CONTRIBUTING.md](../CONTRIBUTING.md)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to take this further. This should be a checkbox at the top of the issue template, and we should take a look at what can be removed from the template once that's added.

@@ -9,6 +9,8 @@ https://petitions.classicpress.net/

For more details about our governance and change planning process, see:
https://www.classicpress.net/democracy/

Please find guidelines on how to contribute to this project in our [CONTRIBUTING.md](../CONTRIBUTING.md)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to take this further. This should be a checkbox at the top of the PR template, and we should take a look at what can be removed from the template once that's added.

@@ -25,7 +27,7 @@ If it fixes an open issue, please link to the issue here.

## How has this been tested?
<!--
Please describe in detail how you tested your changes.
Please describe in detail how you tested your changes. If you have challenges writing tests, please consult the document linked. https://make.wordpress.org/core/handbook/testing/automated-testing/phpunit/ or reach the community on [slack](https://join.slack.com/t/classicpress/shared_invite/enQtNDIwNjY2OTg1MjAxLWJiM2U2NmY3ZjFlZjQ4Zjk2OGI4ZTg3NzY1ZTU3NzI3OTRjMTU0YzAzOWUyZmZlODgyOWE1YTViYjcwY2Y5YzI)
Copy link
Contributor

@nylen nylen Aug 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use https://www.classicpress.net/join-slack/ instead. It redirects to the long invite link, but it's shorter and we can update the invite link later if needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I wonder if this should go in the main contributing document instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea, better add there the petitions link instead (people start pr without check it)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Petitions link doesn't make sense under "How has this been tested". Better to point people to the contribution documentation which explains how the petitions work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spot on

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking as unresolved, this link still needs an update.

README.md Outdated

We are working on several ways to recognize people for their contributions to ClassicPress. In the meantime, you can view a list of code contributions [here on GitHub](https://github.com/ClassicPress/ClassicPress/compare/LAST_WP_COMMIT...develop).
Would you like to help? [Here is how you can start](../documentation/.github/CONTRIBUTING.md).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's switch these two paragraphs ("Would you like to help" before "We are working on...").

Copy link
Contributor

@nylen nylen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is good to go now, I've cleaned up a few more things and also added a section about automated tests. I'm going to go ahead and merge it, because I'd like to link to this from a blog post, but please feel free to leave comments about things that can still be improved further here.

@nylen nylen merged commit dc739de into ClassicPress:develop Aug 11, 2019
@bahiirwa bahiirwa deleted the documentation branch September 14, 2019 12:34
@viktorix viktorix added the exclude For filtering purposes only. label Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude For filtering purposes only.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants