Protect master branch for website

[KirstieJane]

We're already hitting a few crossed wires with folks working on the same topic at the same time. Hopefully we can smooth that out in the long run, but for now I think we should definitely protect the master branch to require at least 1 review before PRs are merged.

What do folks think? What would be reasonable?

[robertoostenveld]

Do we have enough people available for reviewing, and with the right skill set? Reviewing the technical aspects is at the moment non-trivial. AFAIK I am the only one that is checking that it works both locally and on github pages, but even my test setup is non-ideal (since my gh-pages version has a different base_url).

For reviewing page content I agree, but I think that for technical and formatting aspects we should be able to iterate more quickly.

Ideally I would have a way to quickly check formatting of a page during review, and also to test build success/failure, since then it would be possible for reviewers to respond quickly. However, see travis-ci/dpl#991.

[robertoostenveld]

If we protect the gh-pages branch (which is the default and the one being rendered), I do think that we should have a .github/CODEOWNERS file that lists who can be held responsible for a timely review. See https://github.blog/2017-07-06-introducing-code-owners/

This relates to an issue I raised some time ago on slack for the (future) agenda of the steering group: "... who is technically capable of merging and/or who feels responsible for it?".

[KirstieJane]

Yep - I agree - I think this is a conversation for the community to have (beyond the steering group - but we can at least make sure it goes forwards!)

I'm very confident we have the technical skills in the BIDS community, but I'm not confident that those who have the skills know how to engage in the most helpful and constructive way. (As in, I think most people won't engage because they don't know if it's up to them or not!)

[KirstieJane]

Side note - netlify does beautiful PR renderings if you want to move from gh pages (still jekyll)

[sappelhoff]

Side note - netlify does beautiful PR renderings if you want to move from gh pages (still jekyll)

I think getting a rendering of each PR would be a very important feature for us! Merging into our personal forks and checking the preview is suboptimal as Robert points out (comparably high effort, and still a bit error prone).

[robertoostenveld]

From the 21 forks of this repository, there are only 4 that now appear to be configured for online rendering on https://username.github.io/bids-website/.

More general, without forking (or without cloning and installing your own Jekyll stack), it is not possible to do a review that includes visual inspection of the format/layout.

I suspect (and hope) that there will still be quite some formatting changes to come, especially when we add more content, which makes this even more relevant.

[sappelhoff]

@KirstieJane --> to come back to this issue. Would you be fine by adding a restriction similar to in bids-specification?

One approving review required for a merge ... but this being possible to be overridden by an admin?

note: for bids-specification we actually need two approving reviews ... but in practice it often happens that a positive review comes in in text form instead of via the GitHub GUI ... or that a typo is corrected just after an approving review, which dismisses that review from the GitHub GUI.

As a result of this, we fairly often have to make use of the admin rights and merge a PR in order to not bother everybody endlessly with making GitHub happy to also display green lights.

[KirstieJane]

So you'll probably remember from OHBM that I deeply disagree with that decision making and approval process. I think it makes sense in the short term to have the website follow the same guidance, but having more inclusive decision making process is a very high (long term) priority for me as a member of the BIDS community.

[sappelhoff]

I think it makes sense in the short term to have the website follow the same guidance, but having more inclusive decision making process is a very high (long term) priority for me as a member of the BIDS community.

yes, I was only thinking of short term solutions as I thought that was what this issue was about (as you mention in your OP

for now I think we should definitely protect the master branch to require at least 1 review before PRs are merged.

But we certainly don't need to rush anything. I am fine keeping this issue open for discussion, but then it should perhaps be phrased differently

[KirstieJane]

I think for short term putting the same governance procedures in as the main specification makes sense.

[robertoostenveld]

The website showed very little change in 2019 (to the level of it becoming a bit "stale"). An advantage of now having an easier to maintain website- at least something I hope for - is that we might get more people that care about it, and hence more people willing to act as reviewers.

I would welcome using the same process as for bids-standard: the advantage of using the same procedure (possibly with 1 rather than 2 reviewers) is that we only need to explain it once and people in general would better understand how it works.

PS I do believe that bids-standard needs to be very strict (since it may introduce persistent changes/bugs in the spec), whereas the website review could in my eyes be more lenient.