Protect branches on shared repo #213
Conversation
Since there is quite a lot of text, put under actual headings rather than as part of the bullet point list
These instructions mean that on a shared repository the pull request system must be used. This prevents accidentally pushing to the main branch, as well as ensuring people get into the habbit of raising pull requests for their changes.
thomaskileyukaea
changed the title
There was a problem hiding this comment.
Thank for the PR @thomaskileyukaea. The changes look good to me, just the MD rendering's been messed up - the section renders as a quote (a possible leftover from the callout the text was in before) - see below. I am happy to fix this if it is easier.
… a list Co-authored-by: Aleksandra Nenadic <a.nenadic@manchester.ac.uk>
|
@anenadic Thank you for your comments - good spot! I actually hadn't intended to remove the callout box! However, having fixed that, I don't think it looks very good having a callout box that is so long. However, looking at this with a fresh pair of eyes I wonder if the whole addition is a bit out of place! I wonder if it would be better to have an extra on recommended GitHub repo configuration settings. I don't know if there is much else that would go in there - perhaps adding collaborators. What I will do - I will add a commit that simply removes the callout box to this PR, and raise a second PR that moves this content into an extras that is simply linked from here. You can then merge whichever of the two changes you feel is stronger (/make further suggestions!) |
@thomaskileyukaea We initially had a longer section on the development models, then shortened it into a callout (which is also a sort of optional reading for learners) as the episode was becoming too long. With your additions it does look like it would be a big callout/section again so perhaps moving into an 'extras' episode may help to manage the size better and seems sensible (I am also OK for it to remain as is and we can have another review). In terms what else to include in that extras episode (if we go down that route) - adding collaborators is covered separately in https://carpentries-incubator.github.io/python-intermediate-development/41-code-review/index.html#step-1-adding-collaborators-to-a-shared-repository and it is OK to have a smaller extras too. |
3 participants
Previously the course said that on a shared repository everyone can push to the main branch, but people should be encouraged to raise pull requests.
GitHub you can enforce this action by protecting the main branch.
I think the course should be instruction the attendees to do this. Without branch protection, it is very easy to accidentally push to the main branch, which can disrupt other developers, miss out on the advantages CI and code review give you, and create confusion about whether that is ever the right approach.
This change alters the text to instruct attendees how to configure branch protection with sensible defaults, and suggests that they do that.
As the text is no longer suitable for a bullet point (and it was pretty long before!) I also put the content under a header. It might be easiest to review the changes commit by commit.
Replaces #212 (was easier to re-do rather than rebase the changes on #201)