Development process » History » Version 1

Peter Amstutz, 05/28/2020 03:03 PM

1 1 Peter Amstutz
h1. Summary of Development Process
2 1 Peter Amstutz
3 1 Peter Amstutz
{{toc}}
4 1 Peter Amstutz
5 1 Peter Amstutz
h1. Revision control
6 1 Peter Amstutz
7 1 Peter Amstutz
h2. Branches
8 1 Peter Amstutz
9 1 Peter Amstutz
* All development should be done in a branch.  The only exception to this should be trivial bug fixes.  What is trivial enough to not need review is the judgement of the developer, but when in doubt, ask for a review.
10 1 Peter Amstutz
* Each story should be done in its own branch.
11 1 Peter Amstutz
* Branch names are "####-story-summary" where #### is the redmine issue number followed by 3 or 4 words that summarize the story.
12 1 Peter Amstutz
* Make your local branches track the main repository (@git push -u@)
13 1 Peter Amstutz
* Commit regularly, and push your branch to the @git.arvados.org@ at the end of each day.
14 1 Peter Amstutz
* Don't push uninvited changes to other developer's branches.
15 1 Peter Amstutz
** To contribute to another developer's branch, check with them first, or create your own branch ("####-story-summary-ABC" where ABC are your initials) and ask the other developer to merge your branch.
16 1 Peter Amstutz
17 1 Peter Amstutz
h3. Merging
18 1 Peter Amstutz
19 1 Peter Amstutz
Branches should not be merged to master until they are ready (see [[Summary of Development Process#Ready to merge|Ready to merge]] below).
20 1 Peter Amstutz
21 1 Peter Amstutz
# @git remote -v@
22 1 Peter Amstutz
** Make sure your @origin@ is git.arvados.org, not github. *Don't push directly to the github master* branch -- let git.arvados.org decide whether it's OK to push to github.
23 1 Peter Amstutz
# @git checkout master@
24 1 Peter Amstutz
# @git pull --ff-only@
25 1 Peter Amstutz
#* This ensures your master is up to date. Otherwise "git push" below might fail, and you'll be backtracking.
26 1 Peter Amstutz
# @git merge --no-ff branchname@
27 1 Peter Amstutz
#* *The @--no-ff@ part is important!* It ensures there is actually a commit representing this merge. This is your opportunity to record the name of your branch being merged, and the relevant story number. Without it, the git history looks like we all just mysteriously started developing at the tip of your (now unnamed) feature branch.
28 1 Peter Amstutz
#* In your merge commit message, *include the relevant story/issue number* (either "@refs #1234@" or "@closes #1234@").
29 1 Peter Amstutz
# @git push@
30 1 Peter Amstutz
# Look for Jenkins' build results in #arvados or at https://ci.arvados.org .
31 1 Peter Amstutz
32 1 Peter Amstutz
h3. Rejected pushes
33 1 Peter Amstutz
34 1 Peter Amstutz
We have a git hook in place that will reject pushes that do not follow these guidelines.  The goal of these policies is to ensure a clean linear history of changes to master with consistent cross referencing with issue numbers.  These policies apply to the commits listed on "git rev-list --first-parent" when pushing to master, and not to commits on any other branches.
35 1 Peter Amstutz
36 1 Peter Amstutz
If you try to push a (set of) commit(s) that does not pass mustard, you will get a [POLICY] reject message on stdout, which will also list the offending commit. You can use
37 1 Peter Amstutz
38 1 Peter Amstutz
  git commit --amend
39 1 Peter Amstutz
40 1 Peter Amstutz
to update the commit message on your last commit, if that is the offending one, or else you can use 
41 1 Peter Amstutz
42 1 Peter Amstutz
  git rebase --interactive
43 1 Peter Amstutz
44 1 Peter Amstutz
to rebase and fix up a commit message on an earlier commit.
45 1 Peter Amstutz
46 1 Peter Amstutz
h4. All merge commits to master must be from a feature branch into master
47 1 Peter Amstutz
48 1 Peter Amstutz
Merges that go the other way (from master to a feature branch) that get pushed to master as a result of a fast-forward push will be rejected.  In other words:  when merging to master, make sure to use --no-ff.
49 1 Peter Amstutz
50 1 Peter Amstutz
h4. Merges between local and remote master branches will be rejected
51 1 Peter Amstutz
52 1 Peter Amstutz
Merges between local and remote master branches (generally merges created by "git pull") will be rejected, in order to maintain a linear master history.  If this happens, you'll need to reset master to the remote head and then remerge or rebase.
53 1 Peter Amstutz
54 1 Peter Amstutz
h4. Proper merge message format
55 1 Peter Amstutz
56 1 Peter Amstutz
All merge commits to master must include the text "Merge branch 'featurebranch'" or they will be rejected.
57 1 Peter Amstutz
58 1 Peter Amstutz
h4. All commits to master include an issue number or explicitly say no issue #
59 1 Peter Amstutz
60 1 Peter Amstutz
All commits to master (both merges and single parent commits) must
61 1 Peter Amstutz
include the text "refs #", "closes #" or "no issue #" or they will be
62 1 Peter Amstutz
rejected.
63 1 Peter Amstutz
64 1 Peter Amstutz
h4. Avoid broken commit messages
65 1 Peter Amstutz
66 1 Peter Amstutz
Your commit message matches
67 1 Peter Amstutz
68 1 Peter Amstutz
  /Please enter a commit message to explain why this merge is necessary/
69 1 Peter Amstutz
70 1 Peter Amstutz
h2. Commit logs
71 1 Peter Amstutz
72 1 Peter Amstutz
See https://arvados.org/projects/arvados/wiki/Coding_Standards
73 1 Peter Amstutz
74 1 Peter Amstutz
h2. Code review process
75 1 Peter Amstutz
76 1 Peter Amstutz
Code review has high priority! Branches shouldn't sit around for days waiting for review/merge.
77 1 Peter Amstutz
78 1 Peter Amstutz
When your branch is ready for review:
79 1 Peter Amstutz
# Create/update a review task on the story so it looks like this:
80 1 Peter Amstutz
#* subject = "review {branch name}"
81 1 Peter Amstutz
#* state = in progress
82 1 Peter Amstutz
#* assignee is not null
83 1 Peter Amstutz
# Ping your reviewer (during daily standup, via e-mail and/or via chat).
84 1 Peter Amstutz
85 1 Peter Amstutz
Doing a review:
86 1 Peter Amstutz
# We will discuss/assign the review requests at daily stand-up.
87 1 Peter Amstutz
# When you start the review, assign the review task to yourself and move the review task to "in progress" to make sure other people don't duplicate your effort.
88 1 Peter Amstutz
# The recommended process for reviewing diffs for a branch is @git diff master...branchname@.  The reviewer must make sure that their repository is up to date (or use @git diff origin/master...origin/branchname@). Note the 3 dots (not two)
89 1 Peter Amstutz
# After doing a review, write up comments ("fix these problems" or "ready to merge") to the story page, make a note of the git commit revision that was reviewed, assign the review task back to the original developer, and notify the original developer on IRC (or by some other means).
90 1 Peter Amstutz
# The original developer should address any outstanding problems/comments in the code, then write a brief response indicating which points were dealt with or intentionally rejected/not addressed.
91 1 Peter Amstutz
# If the response involves more commits, do that, then goto "branch is ready for review". This process iterates until the branch is deemed ready to merge.
92 1 Peter Amstutz
# Once the branch is merged, move the "review" task to "resolved".
93 1 Peter Amstutz
94 1 Peter Amstutz
To list unmerged branches:
95 1 Peter Amstutz
* Yours: @git branch --no-merged master@
96 1 Peter Amstutz
* Everyone: @git branch -a --no-merged master@
97 1 Peter Amstutz
98 1 Peter Amstutz
h2. Ready to merge
99 1 Peter Amstutz
100 1 Peter Amstutz
When merging, both the developer and the reviewer should be convinced that:
101 1 Peter Amstutz
* Current/recent master is merged. (Otherwise, you can't predict what merge will do.)
102 1 Peter Amstutz
* The branch is pushed to git.arvados.org
103 1 Peter Amstutz
* The code is suitably robust.
104 1 Peter Amstutz
* The code is suitably readable.
105 1 Peter Amstutz
* The code is suitably scalable. For example, client code is not allowed to print or sort unbounded lists. If the code handles a list of items, consider what happens when the list is 10x as large as you expect. What about 100x? A million times?
106 1 Peter Amstutz
* The code accomplishes what the story specified. If not, explain why (e.g., the branch is only part of the story, a better solution was found, etc.) in the issue comments
107 1 Peter Amstutz
* New API names (methods, attributes, error codes) and behaviors are well chosen. It sucks to change them later, and have to choose between compatibility and greatness.
108 1 Peter Amstutz
* Tests that used to pass still pass. (Be extremely careful when altering old tests to make them pass. Do not change existing tests to test new code. Add assertions and write new tests. If you change or remove an existing test, you are breaking behavior that someone already decided was worth testing!)
109 1 Peter Amstutz
* Recent clients/SDKs work against the new API server. (Things rarely turn out well when we rely on all clients being updated at once in lockstep with the API server. Our test suite doesn't check this for us yet, so for now we have to pay attention.)
110 1 Peter Amstutz
* New/fixed behavior is tested. (Although sometimes we decide not to block on inadequate testing infrastructure... that sucks!)
111 1 Peter Amstutz
* New/changed behavior is documented. Search the doc site for relevant keywords to help you find the right sections.
112 1 Peter Amstutz
* Whitespace errors are not committed. (Tab characters, spaces at EOL, etc.)
113 1 Peter Amstutz
* Git commit messages are descriptive (see [[arvados:Coding Standards]]). If they aren't, this is your last chance to rebase/reword.
114 1 Peter Amstutz
115 1 Peter Amstutz
h2. Handling pull requests from github
116 1 Peter Amstutz
117 1 Peter Amstutz
_This is only for contributions by *external contributors*, i.e., people who don't have permission to write directly to arvados.org repositories._
118 1 Peter Amstutz
119 1 Peter Amstutz
First make sure your master is up to date.
120 1 Peter Amstutz
121 1 Peter Amstutz
    git checkout master; git pull --ff-only
122 1 Peter Amstutz
123 1 Peter Amstutz
*Option 1:* On the pull request page on github, click the "You can also merge branches on the command line" link to get instructions.
124 1 Peter Amstutz
125 1 Peter Amstutz
* Don't forget to run tests.
126 1 Peter Amstutz
127 1 Peter Amstutz
*Option 2:* (a bit shorter)
128 1 Peter Amstutz
129 1 Peter Amstutz
Say we have "chapmanb  wants to merge 1 commit into arvados:master from chapmanb:branchname"
130 1 Peter Amstutz
* @git fetch https://github.com/chapmanb/arvados.git branchname:chapmanb-branchname@
131 1 Peter Amstutz
* @git merge --no-ff chapmanb-branchname@
132 1 Peter Amstutz
* Use the commit message: @Merge branch 'branchname' from github.com/chapmanb. No issue #@
133 1 Peter Amstutz
(or @refs #1234@ if there is an issue#)
134 1 Peter Amstutz
* Confirm diff: @git diff origin/master master@
135 1 Peter Amstutz
* Run tests
136 1 Peter Amstutz
* @git push@
137 1 Peter Amstutz
138 1 Peter Amstutz
h1. Non-fast-forward push
139 1 Peter Amstutz
140 1 Peter Amstutz
Please don't get into a situation where this is needed.
141 1 Peter Amstutz
142 1 Peter Amstutz
# On dev box: @git push -f git@github.com:arvados/arvados proper_head_commit:master proper_head_commit:staging@
143 1 Peter Amstutz
# On dev box: @git push -f git@git.arvados.org:arvados.git proper_head_commit:master@
144 1 Peter Amstutz
# As gitsync@dev.arvados.org: @cd /scm/arvados; git fetch origin; git checkout master; git reset --hard origin/master@
145 1 Peter Amstutz
146 1 Peter Amstutz
(At least that's what TC did on 2016-03-10. We'll see how it goes.)
147 1 Peter Amstutz
148 1 Peter Amstutz
h1. Working with external upstream projects
149 1 Peter Amstutz
150 1 Peter Amstutz
Development process summary (1-6 should follow the guidelines above)
151 1 Peter Amstutz
152 1 Peter Amstutz
# Each feature is developed in a git branch named @<issue_number>-<summary>@, for example @12521-web-app-config@
153 1 Peter Amstutz
# Each feature has a "Review" task.  You can see the features and review tasks on the task board.
154 1 Peter Amstutz
# When the feature branch is ready for review, update the title of the Review task to say "Review <branchname>" and move it from the *New* column the to *In Progress* column
155 1 Peter Amstutz
# The reviewer responds on the issue page with questions or comments
156 1 Peter Amstutz
# When the branch is ready to merge, the reviewer will add a comment "Looks Good To Me" (LGTM) on the issue page
157 1 Peter Amstutz
# Merge the feature into into the Arvados main branch
158 1 Peter Amstutz
# Push the feature branch to github and make a pull request (PR) of the branch against the external project upstream
159 1 Peter Amstutz
# Handle code review comments/change requests from the external project team
160 1 Peter Amstutz
# Once the external project merges the PR, merge external project upstream master back into the feature branch
161 1 Peter Amstutz
# Determine if external project upstream brings any unrelated changes that breaks things for us
162 1 Peter Amstutz
# If necessary, make fixes, make a new PR, repeat until stable
163 1 Peter Amstutz
# Merge the feature branch (now up-to-date with external project upstream) into Arvados master
164 1 Peter Amstutz
165 1 Peter Amstutz
This process is intended to let us work independently of how quickly the external project team merges our PRs, while still maximizing the chance that they will be able to accept our PRs by limiting the scope to one feature at a time.
166 1 Peter Amstutz
167 1 Peter Amstutz
This assumes using git merge commits and avoiding rebases, so we can easily perform merges back and forth between the three branches (Arvados master, feature, external project master).
168 1 Peter Amstutz
169 1 Peter Amstutz
170 1 Peter Amstutz
h1. Scrum
171 1 Peter Amstutz
172 1 Peter Amstutz
h2. References
173 1 Peter Amstutz
174 1 Peter Amstutz
These books give us a reference point and vocabulary. 
175 1 Peter Amstutz
176 1 Peter Amstutz
* _Essential Scrum: A Practical Guide to the Most Popular Agile Process_ by Kenneth Rubin
177 1 Peter Amstutz
* _User Stories Applied: For Agile Software Development_ by Mike Cohen
178 1 Peter Amstutz
179 1 Peter Amstutz
h2. Roles
180 1 Peter Amstutz
181 1 Peter Amstutz
182 1 Peter Amstutz
h3. Product Owner
183 1 Peter Amstutz
184 1 Peter Amstutz
* Decide what goes on the backlog
185 1 Peter Amstutz
* Decide backlog priorities
186 1 Peter Amstutz
* Work with stakeholders to understand their requirements and priorities
187 1 Peter Amstutz
* Encode stakeholder requirements/expectations as user stories
188 1 Peter Amstutz
* Lead sprint planning meetings 
189 1 Peter Amstutz
* Lead release planning meetings 
190 1 Peter Amstutz
* Lead product planning meetings 
191 1 Peter Amstutz
* Lead Sprint Kick-off Meetings
192 1 Peter Amstutz
* Lead Sprint Review Meetings
193 1 Peter Amstutz
* Decide on overall release schedule 
194 1 Peter Amstutz
195 1 Peter Amstutz
h3. Scrum Master
196 1 Peter Amstutz
197 1 Peter Amstutz
* Lead Daily Scrum Meeting
198 1 Peter Amstutz
* Help to eliminate road blocks 
199 1 Peter Amstutz
* Lead Sprint Retrospective Meetings
200 1 Peter Amstutz
* Organize Sprint Schedule 
201 1 Peter Amstutz
* Help team organize and stay on track with Scrum process
202 1 Peter Amstutz
* Teach new engineers how Scrum works
203 1 Peter Amstutz
204 1 Peter Amstutz
h3. Top stakeholders
205 1 Peter Amstutz
206 1 Peter Amstutz
* Conduct market research 
207 1 Peter Amstutz
* Synthesize market research into user stories 
208 1 Peter Amstutz
* Work with Product Owner to prioritize stories
209 1 Peter Amstutz
* Define overall business goals for product 
210 1 Peter Amstutz
* Work with Product Owner to define overall release cycle 
211 1 Peter Amstutz
* Organize User Input and dialog with users for engineering team 
212 1 Peter Amstutz
* Contribute to backlog grooming 
213 1 Peter Amstutz
* Bring voice of customer into planning process 
214 1 Peter Amstutz
* Define user personas 
215 1 Peter Amstutz
* Coordinate user communication 
216 1 Peter Amstutz
* Develop technical marketing and sales materials 
217 1 Peter Amstutz
* Assist sales team in presenting product value proposition
218 1 Peter Amstutz
* Train sales in technical aspects of the product
219 1 Peter Amstutz
220 1 Peter Amstutz
h2. Definition of Done
221 1 Peter Amstutz
222 1 Peter Amstutz
An issue is resolved when:
223 1 Peter Amstutz
224 1 Peter Amstutz
* Code is written
225 1 Peter Amstutz
* Existing code is refactored if appropriate
226 1 Peter Amstutz
* Documentation is written/updated
227 1 Peter Amstutz
* Acceptance tests are satisfied
228 1 Peter Amstutz
* Code is merged in master
229 1 Peter Amstutz
* All Jenkins jobs pass (test, build packages, deploy to dev clusters)
230 1 Peter Amstutz
* Feature works on applicable dev clusters
231 1 Peter Amstutz
232 1 Peter Amstutz
h2. Standard Schedule
233 1 Peter Amstutz
234 1 Peter Amstutz
Sprints are two weeks long. They start and end on Wednesdays.
235 1 Peter Amstutz
236 1 Peter Amstutz
h3. Key meetings
237 1 Peter Amstutz
238 1 Peter Amstutz
Every day at 11:45am:
239 1 Peter Amstutz
240 1 Peter Amstutz
Daily Scrum (15 Minutes)
241 1 Peter Amstutz
Who: Development team, product owner. Silent observers welcome.
242 1 Peter Amstutz
* What did you do yesterday?
243 1 Peter Amstutz
* What will you do today?
244 1 Peter Amstutz
* What obstacles are in your way?
245 1 Peter Amstutz
246 1 Peter Amstutz
h4. Sprint review & kickoff (every 2 weeks on Wednesday):
247 1 Peter Amstutz
248 1 Peter Amstutz
Sprint Review (30 minutes)
249 1 Peter Amstutz
Who: Development team, product owner, stakeholders.
250 1 Peter Amstutz
* Demo of each feature built and relationship to stories
251 1 Peter Amstutz
* Product owner explains which backlog items are done
252 1 Peter Amstutz
* Development team demonstrates the work done, and answers questions about the sprint increment
253 1 Peter Amstutz
* Product owner discusses the backlog as it stands. Revise expected completion dates based on recent progress (if needed)
254 1 Peter Amstutz
* Review current product status in context of business goals
255 1 Peter Amstutz
256 1 Peter Amstutz
Sprint Retrospective (30 minutes)
257 1 Peter Amstutz
Who: Development team, product owner.
258 1 Peter Amstutz
* Review what processes worked well, and what didn't, in the sprint just finished
259 1 Peter Amstutz
* Propose and agree to changes to improve future sprints
260 1 Peter Amstutz
* Assign action items (meetings/tasks) to implement agreed-upon process improvements
261 1 Peter Amstutz
262 1 Peter Amstutz
Sprint Kick Off (1 hour)
263 1 Peter Amstutz
Who: Development team, product owner.
264 1 Peter Amstutz
* Add latest bugs or dependencies to sprint
265 1 Peter Amstutz
* Create tasks for each story
266 1 Peter Amstutz
* Assign a developer to each task
267 1 Peter Amstutz
* Assign an on-call engineer for that sprint who will triage customer support requests
268 1 Peter Amstutz
* Check that commitment level is realistic
269 1 Peter Amstutz
270 1 Peter Amstutz
h4. Planning (alternate Wednesdays mid-sprint)
271 1 Peter Amstutz
272 1 Peter Amstutz
Roadmap review (30 minutes)
273 1 Peter Amstutz
Who: Development team, product owner, stakeholders.
274 1 Peter Amstutz
* Clarify stories
275 1 Peter Amstutz
* Get user input or identify user questions
276 1 Peter Amstutz
* Describe acceptance tests
277 1 Peter Amstutz
* Prioritize against other work
278 1 Peter Amstutz
279 1 Peter Amstutz
Sprint Planning (1 hours)
280 1 Peter Amstutz
Who: Development team, product owner.
281 1 Peter Amstutz
* Discuss and get consensus on features design & implementation strategy
282 1 Peter Amstutz
* Estimate story points (1 point is an ideal developer day)
283 1 Peter Amstutz
* Assign to sprint