docs: contribution guideline #390
Conversation
JP-Ellis
added
smartbear-supported
This commit rewrites the contributing.md file to be up to date with the current state of the project. It draws heavily from the `CONTRIBUTING.md` file in the [Docusaurus](https://github.com/facebook/docusaurus) project. Signed-off-by: JP-Ellis <[email protected]>
Along with the overhaul of the `CONTRIBUTING.md` file, this commit adds issue and PR templates to the repository. These templates have been heavily inspired by the [Docusaurus](https://github.com/facebook/docusaurus) project. Signed-off-by: JP-Ellis <[email protected]>
JP-Ellis
force-pushed
the
docs/contributing-md
branch
from
e0ae18a to
23ed629
Compare
| - An executable code example where possible. You can fork this repository and | ||
| use the [examples] directory to quickly recreate your issue. | ||
| - [How to Contribute to Open Source](https://opensource.guide/how-to-contribute/) | ||
| - [Building Welcoming Communities](https://opensource.guide/building-community/) |
There was a problem hiding this comment.
We've got a few pages on the main website with regards to contributing, so maybe better to link to those, and add to them where appropriate, rather than do external docs, wdyt?
- General contributing
- Where users can help
- Contributing to the code
- CI testing
There was a problem hiding this comment.
There is also a devrel repo as well
https://github.com/pact-foundation/devrel
which was previously named readme which can serve as a central hub
Sorry for the link spam! Not necessarily asking you to action anything off this, but just to consider, provide a map of some of things and get your thoughts on how we can corale this to make a more cohesive experience not just here in pact-python but provide a similar experience in places people expect (being a newbie helps bring fresh perspective :) )
There was a problem hiding this comment.
I found the opensource.guide links to be quite good and general introduction to contributing to open source, but definitely I need to add the docs.pact.io/contributing links! I'm not sure yet how to incorporate the devrel repo, as it isn't as clear what the user should do once they get there.
There was a problem hiding this comment.
I found the opensource.guide links to be quite good and general introduction to contributing to open source, but definitely I need to add the docs.pact.io/contributing links!
I will give them a read! I am sure they will be useful with others, and its something that resonated with you it will with others.
I'm not sure yet how to incorporate the devrel repo, as it isn't as clear what the user should do once they get there.
Yeah it's not really a home, or a springboard to anything really, just a bit of copy pasta from the docs, possibly for users who aren't using the docs website.
Open to suggestions and ways we can improve it! or if it is even useful!
CONTRIBUTING.md
Outdated
|
|
||
| ## Running the tests | ||
| Pact Python has one primary branch `master` and we use feature branches to deliver new features with pull requests. We use the following naming convention for branches: |
There was a problem hiding this comment.
We probably want to have the conversation about master/main and implications across the estate.
I'm okay with it being master, but appreciate others reservations. My only consideration with regards to programming is for consistency.
New repos are created with main by default, so at some point we should probably migrate to main across older repos.
We call it a main branch detection property in the pact broker
https://docs.pact.io/pact_broker/branches#automatic-main-branch-detection
To assist in the migration from tags to branches, the main branch will be automatically set for a pacticipant if a version is created with a branch or tag name matching one of develop, main, or master.
so from a code perspective, we support both, so its just a question on if and when as maintainers, we want to make the switch.
Bit of an aside from here, but worth calling it.
Another note, it is a protected branch, and cannot be merged to without at least one approver, the same goes fo for the release process, although that is documented seperately
There was a problem hiding this comment.
I completely agree that we should migrate to main. Most platforms have migrated to having main as the default, and I believe Git even has that as the default now.
I believe there's also a distinction between casually referring to the main branch vs the main branch. In the former, this could be anything (e.g., develop or stable, or it could be v2-main), whereas in the latter, we are explicitly referring to the branch called main. Admittedly, this is a bit of a nit-pick.
The problem is that changing from master to main will break things for people, and I would rather not do this right now.
At this stage, I have documented what we are currently using, and if/when we migrate master to main, the docs can be updated.
As for the merging process, I think that's fine. It's pretty self-explanatory once the PR is created.
CONTRIBUTING.md
Outdated
|
|
||
| ## Pull Requests | ||
|
|
||
| So you have decided to contribute code back to upstream by opening a pull request. You've invested a good chunk of time, and we appreciate it. We will do our best to work with you and get the PR looked at. |
There was a problem hiding this comment.
they may not have invested the time yet and are just reading the docs on how to contribute.
Making a pull request can often feel daunting but it should need to be. We are appreciate anyone experimenting with the code, but we greatly appreciate upstream changes as these assist maintainers and can deliver real value to end users.
I would maybe put something in about considering if the change it useful for a subset of people, or all users, and considering should be paid to documenting a feature. If someone hasn't already, is it worth them filling out a feature request issue template (rather than having to hand roll their own desc)
Also would the feature benefit just pact-python, or the wider pact ecosystem. (and considerations for the implications)
There was a problem hiding this comment.
they may not have invested the time yet and are just reading the docs on how to contribute.
Very true! Let me fix this up and make it clear that before a PR is made, a ticket should ideally have been created first (which then guides them through the ticket workflow and iron out some kinks).
CONTRIBUTING.md
Outdated
|
|
||
| All pull requests should be opened against the `master` branch. | ||
|
|
||
| We have a lot of integration systems that run automated tests to guard against mistakes. The maintainers will also review your code and fix obvious issues for you. These systems' duty is to make you worry as little about the chores as possible. Your code contributions are more important than sticking to any procedures, although completing the checklist will surely save everyone's time. |
There was a problem hiding this comment.
The maintainers will also review your code and fix obvious issues for you.
would be fix obvious errors? or generally would be point them out?
do they need to provide write access to their branch, if we are going to fix things.
I think it is okay for us to point these out, and they correct them as part of the learning process
There was a problem hiding this comment.
If they open a PR, I think they have an option to automatically grant us write access to the PR. So if it is something really trivial, we could easily go in and fix it and that's sometimes faster than asking them to fix up every minute detail. I think it also depends on the maintainer and the contributor.
Signed-off-by: JP-Ellis <[email protected]>
|
Wow @YOU54F! You are a machine. Thank you so much for all of the comments! I have incorporated them into the PR. Let me know what you think 🚀 |
Thanks buddy, appreciate that was a tonne of comments, I really should start a review, rather than individual comments, as it may stop you getting flooded with notifications. Looking forward to reviewing again |
|
Thanks again @YOU54F! I will leave this open for a few more days for any further feedback, @mefellows did you want to have a read over this? |
|
I think let's get this in and tweak, it is clearly better than what we have, easily updated after and Yousaf has cross-examined it like a Lawyer needing a few extra dollars before xmas 😆 . |
|
Last comment, I noticed https://github.com/orgs/pact-foundation/projects/20 - should we consider adding that to the developer guide so people can see what's going on? |
|
I've started using this board as a way to track the work within this repo, and if it is useful, it can be leveraged across the broader pact-foundation repositories. Given it is still a work in progress, let's add this in a little later once I've ironed out kinks :) |
This PR overhauls the
contributing.mdfile and also adds templates for GitHub issues. The changes are mainly taken from the (excellent) Docusaurus repository.I realised while pushing the changes that I should have done a more extensive look at the other
pact-*repos and try and ensure consistency. So happy to change things up as need be.When it comes to reviewing the GitHub issue templates which combine YAML and Markdown, a semi-rendered version can be seen when browsing the branch:
bug.ymlandfeature.yml.