Skip to content

Instantly share code, notes, and snippets.

@cvalarida
Last active April 12, 2017 19:47
Show Gist options
  • Save cvalarida/11c12f7ba5e15c9d96555d08d3502037 to your computer and use it in GitHub Desktop.
Save cvalarida/11c12f7ba5e15c9d96555d08d3502037 to your computer and use it in GitHub Desktop.
Workflow Procedures

Workflow Procedures

For lack of a better name, we'll call it that. For now.

Abstract

This document outlines the procedure in which work makes it from start to finish, merging into master. We'll be looking at it from a pretty high level and skimming over a lot as we go.

Assumptions

  • master is clean
    • All code that makes it into master has been reviewed by all necessary parties and it's golden
    • Code in master ends up in production automagically and that process shouldn't be interfered

Process

To get from the amorphous idea that work needs to get done to a finished product, we have to go through a process.

Step One: Ticket Creation

The first thing that happens before work is done is the creation of a ticket which outlines the work. This should have a descriptive, brief title encompassing the scope of the work. A description should be included for more complicated tasks that need more than a short sentence of explanation. Labels are added to categorize the work so it can be easily found later.

This can be done in multiple phases such as backlog grooming, but that's outside the scope of this document.

No surprises so far.

Step Two: Ticket Prioritization

At the sprint planning meeting (or before) is when this happens. Tickets are moved from Backlog into Ready, Ready into Current Sprint, and ordered. Tasks that need to be done first (have a higher priority) go to the top of their respective swimlanes.

Step Three: Ticket Assignment

Because somebody's gotta work. This step may be optional if the team is operating on a first-come-first-serve basis.

Step Four: Work, Work, Work

Make stuff happen. Pull off the tickets from the top of the Current Sprint column that you can work on and move them into In Progress. These are either assigned to you or are in your "group" (e.g. front end tickets that any FE engineer can work on).

At this point, work goes in a local branch which may or may not be pushed to origin, but there is no PR for it.

Let's say you're working on something and need feedback before continuing. Just make a PR with "[WIP]" prepended to the title to show that it's not ready for a full review.

Step Five: Review

When all work has been done and needs to be reviewed, make a PR for the branch. In the description "connect" it to the original ticket, them move the ticket into Validate. If there was an existing PR, remove the "[WIP]" from the title.

Assign one or more people to review your branch. If desired, you may also explicitly request reviews. As a rule, all requested reviewers must review it before the branch is merged, so use this sparingly.

Product person review

Outward-facing tickets should get reviewed by a product person. A product person may be a designer, product manager, content editor, or somebody else. This includes user-facing pages as well as back-end changes that affect what data is being sent to external systems.

Step Six: Merge and Cleanup

After review, the branch is finally ready for merging! (If it looks good to both parties, of course.) Resolve any merge conflicts and merge that sucker! Delete the branch, and move the ticket into Done.

Congratulations! Your work here is done.

Miscellaneous Minutiae

Random tidbits of clarification.

Assigning vs requestiong review

Assign a group of people to review a PR if any in a group can do so. For example, a typical front end PR may be assigned to two or three other front end devs. The first dev to have time reviews it. (Really you can assign whatever number, but be conscientious of everybody's workloads and try to minimize the load.)

Request reviews of people who absolutely must be a part of the reviewing process before the branch gets merged into master. Use this if a design has to be signed off by a designer or the product manager has to verify specific wording for a user-facing page.

Product person definition

I actually don't know who all qualifies as a "product person," so I'm using that term generically until I get clarification. Basically, this is somebody who knows what the end-goal is and is probably not a developer.

Post review process

If there's a lot of work to do after a branch has gone through a review, consider moving the ticket back to In Progress. When it's ready for another review, you can move it back. This keeps the Validate column clean.

Technical vs product person reviews

There are times where reviews from only product people or only developers are needed. Use your judgement on who to assign or request reviews from. If your PR is for writing unit tests, it probably doesn't need to go through a product person. If your PR is a content-only change, it probably doesn't need to go through another dev.

Iterative reviews

There are times where the first thing you push out isn't what's desired after all. For these times, once you fix up the "troubled areas," ping the reviewer and ask for another look. This will help keep you from being blocked while waiting for a review.

@raquelromano-gov
Copy link

Pasting in a brief slack discussion from while you were out sick. Might be easier to iterate (and comment inline) if we turn this into a PR.


Alex Yale-Loehr [7:47 AM]
Catching up on reading this and wanted to add a couple things:

  1. I had a TODO to draft up what validate meant but it hadn’t happened yet. I think you (meaning Chris and Raquel from the conversation above) are in the best position to draft this, so I fully support either/both of you doing this and will remove it from my TODO list. Thank you!

Raquel Romano [8:24 AM]
Chris started a "Workflow Procedures" draft yesterday (not mentioning him b/c I think he is sick and don't want to bug him); just wondering if this is needed vets.gov-wide, or just for Rainbows, and how/whether it should co-exist or be merged into https://github.com/department-of-veterans-affairs/vets.gov-team/blob/master/Work%20Practices/How%20to%20File%20Bugs%20Tasks%20and%20Questions.md
since that document talks about workflow and waffle columns


Alex Yale-Loehr [8:41 AM]
i think having Rainbows come up with the procedure that works for you first, testing it out over ~1 week (and iterating as necessary), then pushing it vets.gov wide will be the way to go. (edited)
So yes to it getting into that document!

@raquelromano-gov
Copy link

raquelromano-gov commented Apr 7, 2017

Finally getting around to commenting:

  1. Ticket creation
  • this section (or the prioritization section) should include a link to How to File Bugs, Tasks, and Questions, which goes into more detail about the waffle board (but is also out of date)
  • that doc should probably emphasize how important labels are now that all issues are being filed on vets.gov-team (it talks about waffle column labels, but not the many other labels we rely on for sanity)
  1. Prioritization
  • it's not clear to me that we've been keeping high-priority things higher in the waffle board (esp. because we're not using priority labels); and when I move something from current to in-progress to validate to done, I usually just dump it wherever waffle's UI offers; maybe it's fine for this to be somewhat organic? not sure
  1. Assignment
  • generally, I've been waiting to self-assign something until I'm ready to work on it, so I assign and immediately move to in-progress; not sure if that's what everyone does; e.g., someone could assign me something and leave it Ready or Current Sprint, and I could just move it to In Progress when I'm ready to work on it. Not sure this matters so feel free to ignore.
  1. Work
  1. Technical Review
  • we should add a link to DS@VA Code Review Norms, which is a good list of norms
  • when I first got here, I didn't know that "connect " or "closes " were what magically made PRs appear on waffle attached to their issue, so it might be nice to mention that (maybe in the above doc, not this one)
  1. Product Review - 7. Merge
  • this is the hairy one that I think we still haven't figured out
  • it's a bit awkward to require product (or UX) to approve a PR when the visible changes may not be obvious from the code changes, even with our screenshots included
    • often product/UX aren't monitoring the Validate column, so an unmerged PR will get stale, requiring the dev to merge or rebase, and reducing productivity
    • it would make more sense for product/UX to look at a demo and play with it to quickly verify it's WAI, but that's only possible if it's merged and pushed to staging; however, then the fix could go out with the next production deployment without sign-off
    • either way, the product review is needed ASAP but product/UX are typically pretty busy with other stuff and might let a day go by without validating
  • I don't have a solution to this! think we should ask Josh Q. and Alex T. for their input

I like the miscellaneous minutiae--some overlap with DS@VA Code Review Norms though, so may want to point these to each other

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment