Quansight-Labs / jupyter-a11y-mgmt

πŸš€β™ΏοΈ Project management for CZI Jupyter accessibility grant
https://jupyter-a11y.netlify.app
BSD 3-Clause "New" or "Revised" License
1 stars 2 forks source link

RFD - Clean-up git flow #120

Closed trallard closed 2 years ago

trallard commented 2 years ago
Status Accepted βœ…
Author(s) @trallard
Date Created 09-05-2022
Date Last updated 09-05-2022
Decision deadline 09-05-2022

Improved git/GitHub flow for contributions

Summary

We need to standardise our git-flow to make collaboration and work more efficient. The current "workflow" (which is none) is extremely confusing and hard to keep track of what folks are working on

User benefit

  1. Increased transparency -> we all know what we are working on
  2. Enhanced collaboration -> if we have a single source of truth we all know what we are doing and how we can help each other
  3. Does not mess up with our local repos (mine are a mess)

Design Proposal

I am proposing the following workflow:

a11y-git-flow

  1. All of the CZI related work will be done on/from Quansight/accessibility fork.
  2. If you are working on a new task/feature/something you make a new branch on this repo (using descriptive names and pre-prended with your GitHub handle) i.e trallard/gitpod-fixes
  3. Work directly on your new branch and add changes - as soon as you start working on your task create a WIP/draft PR against the repo
  4. Once finished: mark as ready and request a review πŸ‘€
  5. If approved and merged -> open a PR against jupyter/accessibility

Alternatives or approaches considered (if any)

More complex workflows - like Git flow (strict) or keep living in chaos

Best practices

Detailed in the previous sections.

Steps needed

  1. Clone https://github.com/Quansight-Labs/accessibility
  2. Migrate whatever you are working on to a branch against ☝🏽 following the user/task patter
  3. Open a PR - WIP if needed
gabalafou commented 2 years ago

This is a great idea!

It seems to me that the central innovation in this RFC is to have all of our feature work in one easy-to-find place on Githubβ€”i.e., github.com/Quansight-Labs/accessibility/pulls.

My one question is whether this would be more easily achieved by us adopting a policy of opening draft PRs against jupyter/accessibility whenever we start a new branch of work, as opposed to working on the branch without a draft PR.

The reason I ask is that with the git workflow that you are proposing it seems to me that we could potentially end up in a situation where somebody opens a PR from Quansight-Labs/accessibility:main -> jupyter/accessibility:main, then somebody else merges a PR into Quansight-Labs/accessibility:main, accidentally adding commits to the open PR on jupyter/accessibility that don't belong there.

So my main question is whether it would be in some ways simpler while still achieving the main goal of transparency if we simply always opened a draft PR against jupyter/accessibility at the beginning, rather than opening a PR towards the end.

But I'm also totally happy to adopt this idea :)

isabela-pf commented 2 years ago

I'm trying to think of any problems this could create, but I haven't been able to yet. So I think this works for me. I'm on board with having clear expectations so we know where to look for each other's work.

Just to clarify, the process is for us all to work of the quansight-labs/accessibility fork of jupyter/accessibility with branches named name/task. Since the visual for all parts of this is the same, I wanted to make sure that the pink section is a branch and not another repo.

Question: Are we voting by reacting to the initiating comment, or waiting for a voting comment. Since there's already a reaction to the initiating comment, I wasn't sure.

Responding to @gabalafou's question

So my main question is whether it would be in some ways simpler while still achieving the main goal of transparency if we simply always opened a draft PR against jupyter/accessibility at the beginning, rather than opening a PR towards the end.

I think this is a good question I'd be interested in hearing all y'all's thoughts. If we want to open PRs in the beginning, I think we could always treat that draft as a working PR that gets closed before opening a more polished one. I don't know if there's any problem with this approach, but I've definitely had to do it before.

trallard commented 2 years ago

@Quansight-Labs/czi-a11y-grant vote on the original comment please πŸ™πŸ½

trallard commented 2 years ago

Notes on shifting left - will move to another issue:

gabalafou commented 2 years ago

From this morning's video call, my takeaway is that we'll want to create a separate repo not only to consolidate our working branches, but also to give us a place to develop and test out Github Actions without being held back by the restrictions on the jupyter/accessibility repo, so... I'm +1 on this

gabalafou commented 2 years ago

@trallard, instead of having to create two separate pull requests for each feature we work on, from Quansight-Labs to Jupyter, would the following workflow work?

  1. Start work by creating branch on Quansight-Labs. For example, Quansight-Labs:gabalafou/axe-gitpod
  2. Immediately open draft PR against the main branch of jupyter/accessibility
  3. When ready for review, take the PR out of draft mode
  4. When reviewed, merge into jupyter/accessibility main branch
  5. Pull updates from jupyter/accessibility to Quansight-Labs/accessibility
gabalafou commented 2 years ago

My main concern is that important conversations related to a code change might get split between the PR in Quansight-Labs versus the PR in jupyter/accessibility. From an end user perspective, I think it's less confusing to future visitors of the jupyter/accessibility repo if the code changes live side-by-side with the PRs and discussions.

Imagine you're exploring the jupyter/accessibility repo. You git blame a file. You see that the changes were introduced by a particular PR. You go to that PR but you see no discussion about the code changes because all of that discussion happened in the PR on the Quansight-Labs repo.

gabalafou commented 2 years ago

In terms of your diagram, this is what I'm proposing:

Github workflow diagram, modified so that pull requests go directly from the fork to the original repo rather than having to pass through a PR against the fork first

trallard commented 2 years ago

We can do that - I am thinking for internal experimentation and review this would add less noise to the main accessibility repo

Also as long as we do not use squash commit for our merge all the history will be preserved even if we do a subsequent pr from labs to Jupyter thus the git blame will be intact and they can easy track to the pr discussion

I know this is the case because I have used this before

gabalafou commented 2 years ago

As I understand it, in the balance: extra overhead for us versus noise on the jupyter/accesibility repo.

Personally, I tend to squash my commit history at the end of a PR because I tend to have a very messy commit history when working on a feature 😁

trallard commented 2 years ago

Maybe room for improvement?