Development process » History » Version 5

Ward Vandewege, 05/12/2021 08:21 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 4 Nico César
#* In your merge commit message, *include Arvados-DCO-1.1-Signed-off-by line* (i.e. Arvados-DCO-1.1-Signed-off-by: Jane Doe <jane@example.com>)
30 1 Peter Amstutz
# @git push@
31 3 Peter Amstutz
# Look for Jenkins' build results at https://ci.arvados.org .
32 1 Peter Amstutz
33 1 Peter Amstutz
h3. Rejected pushes
34 1 Peter Amstutz
35 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.
36 1 Peter Amstutz
37 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
38 1 Peter Amstutz
39 1 Peter Amstutz
  git commit --amend
40 1 Peter Amstutz
41 1 Peter Amstutz
to update the commit message on your last commit, if that is the offending one, or else you can use 
42 1 Peter Amstutz
43 1 Peter Amstutz
  git rebase --interactive
44 1 Peter Amstutz
45 1 Peter Amstutz
to rebase and fix up a commit message on an earlier commit.
46 1 Peter Amstutz
47 1 Peter Amstutz
h4. All merge commits to master must be from a feature branch into master
48 1 Peter Amstutz
49 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.
50 1 Peter Amstutz
51 1 Peter Amstutz
h4. Merges between local and remote master branches will be rejected
52 1 Peter Amstutz
53 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.
54 1 Peter Amstutz
55 1 Peter Amstutz
h4. Proper merge message format
56 1 Peter Amstutz
57 1 Peter Amstutz
All merge commits to master must include the text "Merge branch 'featurebranch'" or they will be rejected.
58 1 Peter Amstutz
59 1 Peter Amstutz
h4. All commits to master include an issue number or explicitly say no issue #
60 1 Peter Amstutz
61 1 Peter Amstutz
All commits to master (both merges and single parent commits) must
62 1 Peter Amstutz
include the text "refs #", "closes #" or "no issue #" or they will be
63 1 Peter Amstutz
rejected.
64 1 Peter Amstutz
65 1 Peter Amstutz
h4. Avoid broken commit messages
66 1 Peter Amstutz
67 1 Peter Amstutz
Your commit message matches
68 1 Peter Amstutz
69 1 Peter Amstutz
  /Please enter a commit message to explain why this merge is necessary/
70 1 Peter Amstutz
71 1 Peter Amstutz
h2. Commit logs
72 1 Peter Amstutz
73 5 Ward Vandewege
See https://dev.arvados.org/projects/arvados/wiki/Coding_Standards
74 1 Peter Amstutz
75 1 Peter Amstutz
h2. Code review process
76 1 Peter Amstutz
77 1 Peter Amstutz
Code review has high priority! Branches shouldn't sit around for days waiting for review/merge.
78 1 Peter Amstutz
79 1 Peter Amstutz
When your branch is ready for review:
80 1 Peter Amstutz
# Create/update a review task on the story so it looks like this:
81 1 Peter Amstutz
#* subject = "review {branch name}"
82 1 Peter Amstutz
#* state = in progress
83 1 Peter Amstutz
#* assignee is not null
84 1 Peter Amstutz
# Ping your reviewer (during daily standup, via e-mail and/or via chat).
85 1 Peter Amstutz
86 1 Peter Amstutz
Doing a review:
87 1 Peter Amstutz
# We will discuss/assign the review requests at daily stand-up.
88 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.
89 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)
90 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).
91 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.
92 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.
93 1 Peter Amstutz
# Once the branch is merged, move the "review" task to "resolved".
94 1 Peter Amstutz
95 1 Peter Amstutz
To list unmerged branches:
96 1 Peter Amstutz
* Yours: @git branch --no-merged master@
97 1 Peter Amstutz
* Everyone: @git branch -a --no-merged master@
98 1 Peter Amstutz
99 1 Peter Amstutz
h2. Ready to merge
100 1 Peter Amstutz
101 1 Peter Amstutz
When merging, both the developer and the reviewer should be convinced that:
102 1 Peter Amstutz
* Current/recent master is merged. (Otherwise, you can't predict what merge will do.)
103 1 Peter Amstutz
* The branch is pushed to git.arvados.org
104 1 Peter Amstutz
* The code is suitably robust.
105 1 Peter Amstutz
* The code is suitably readable.
106 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?
107 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
108 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.
109 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!)
110 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.)
111 1 Peter Amstutz
* New/fixed behavior is tested. (Although sometimes we decide not to block on inadequate testing infrastructure... that sucks!)
112 1 Peter Amstutz
* New/changed behavior is documented. Search the doc site for relevant keywords to help you find the right sections.
113 1 Peter Amstutz
* Whitespace errors are not committed. (Tab characters, spaces at EOL, etc.)
114 1 Peter Amstutz
* Git commit messages are descriptive (see [[arvados:Coding Standards]]). If they aren't, this is your last chance to rebase/reword.
115 1 Peter Amstutz
116 1 Peter Amstutz
h2. Handling pull requests from github
117 1 Peter Amstutz
118 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._
119 1 Peter Amstutz
120 1 Peter Amstutz
First make sure your master is up to date.
121 1 Peter Amstutz
122 1 Peter Amstutz
    git checkout master; git pull --ff-only
123 1 Peter Amstutz
124 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.
125 1 Peter Amstutz
126 1 Peter Amstutz
* Don't forget to run tests.
127 1 Peter Amstutz
128 1 Peter Amstutz
*Option 2:* (a bit shorter)
129 1 Peter Amstutz
130 1 Peter Amstutz
Say we have "chapmanb  wants to merge 1 commit into arvados:master from chapmanb:branchname"
131 1 Peter Amstutz
* @git fetch https://github.com/chapmanb/arvados.git branchname:chapmanb-branchname@
132 1 Peter Amstutz
* @git merge --no-ff chapmanb-branchname@
133 1 Peter Amstutz
* Use the commit message: @Merge branch 'branchname' from github.com/chapmanb. No issue #@
134 1 Peter Amstutz
(or @refs #1234@ if there is an issue#)
135 1 Peter Amstutz
* Confirm diff: @git diff origin/master master@
136 1 Peter Amstutz
* Run tests
137 1 Peter Amstutz
* @git push@
138 1 Peter Amstutz
139 1 Peter Amstutz
h1. Non-fast-forward push
140 1 Peter Amstutz
141 1 Peter Amstutz
Please don't get into a situation where this is needed.
142 1 Peter Amstutz
143 1 Peter Amstutz
# On dev box: @git push -f git@github.com:arvados/arvados proper_head_commit:master proper_head_commit:staging@
144 1 Peter Amstutz
# On dev box: @git push -f git@git.arvados.org:arvados.git proper_head_commit:master@
145 1 Peter Amstutz
# As gitsync@dev.arvados.org: @cd /scm/arvados; git fetch origin; git checkout master; git reset --hard origin/master@
146 1 Peter Amstutz
147 1 Peter Amstutz
(At least that's what TC did on 2016-03-10. We'll see how it goes.)
148 1 Peter Amstutz
149 1 Peter Amstutz
h1. Working with external upstream projects
150 1 Peter Amstutz
151 1 Peter Amstutz
Development process summary (1-6 should follow the guidelines above)
152 1 Peter Amstutz
153 1 Peter Amstutz
# Each feature is developed in a git branch named @<issue_number>-<summary>@, for example @12521-web-app-config@
154 1 Peter Amstutz
# Each feature has a "Review" task.  You can see the features and review tasks on the task board.
155 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
156 1 Peter Amstutz
# The reviewer responds on the issue page with questions or comments
157 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
158 1 Peter Amstutz
# Merge the feature into into the Arvados main branch
159 1 Peter Amstutz
# Push the feature branch to github and make a pull request (PR) of the branch against the external project upstream
160 1 Peter Amstutz
# Handle code review comments/change requests from the external project team
161 1 Peter Amstutz
# Once the external project merges the PR, merge external project upstream master back into the feature branch
162 1 Peter Amstutz
# Determine if external project upstream brings any unrelated changes that breaks things for us
163 1 Peter Amstutz
# If necessary, make fixes, make a new PR, repeat until stable
164 1 Peter Amstutz
# Merge the feature branch (now up-to-date with external project upstream) into Arvados master
165 1 Peter Amstutz
166 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.
167 1 Peter Amstutz
168 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).
169 1 Peter Amstutz
170 1 Peter Amstutz
171 1 Peter Amstutz
h1. Scrum
172 1 Peter Amstutz
173 1 Peter Amstutz
h2. References
174 1 Peter Amstutz
175 1 Peter Amstutz
These books give us a reference point and vocabulary. 
176 1 Peter Amstutz
177 1 Peter Amstutz
* _Essential Scrum: A Practical Guide to the Most Popular Agile Process_ by Kenneth Rubin
178 1 Peter Amstutz
* _User Stories Applied: For Agile Software Development_ by Mike Cohen
179 1 Peter Amstutz
180 1 Peter Amstutz
h2. Roles
181 1 Peter Amstutz
182 1 Peter Amstutz
183 1 Peter Amstutz
h3. Product Owner
184 1 Peter Amstutz
185 1 Peter Amstutz
* Decide what goes on the backlog
186 1 Peter Amstutz
* Decide backlog priorities
187 1 Peter Amstutz
* Work with stakeholders to understand their requirements and priorities
188 1 Peter Amstutz
* Encode stakeholder requirements/expectations as user stories
189 1 Peter Amstutz
* Lead sprint planning meetings 
190 1 Peter Amstutz
* Lead release planning meetings 
191 1 Peter Amstutz
* Lead product planning meetings 
192 1 Peter Amstutz
* Lead Sprint Kick-off Meetings
193 1 Peter Amstutz
* Lead Sprint Review Meetings
194 1 Peter Amstutz
* Decide on overall release schedule 
195 1 Peter Amstutz
196 1 Peter Amstutz
h3. Scrum Master
197 1 Peter Amstutz
198 1 Peter Amstutz
* Lead Daily Scrum Meeting
199 1 Peter Amstutz
* Help to eliminate road blocks 
200 1 Peter Amstutz
* Lead Sprint Retrospective Meetings
201 1 Peter Amstutz
* Organize Sprint Schedule 
202 1 Peter Amstutz
* Help team organize and stay on track with Scrum process
203 1 Peter Amstutz
* Teach new engineers how Scrum works
204 1 Peter Amstutz
205 1 Peter Amstutz
h3. Top stakeholders
206 1 Peter Amstutz
207 1 Peter Amstutz
* Conduct market research 
208 1 Peter Amstutz
* Synthesize market research into user stories 
209 1 Peter Amstutz
* Work with Product Owner to prioritize stories
210 1 Peter Amstutz
* Define overall business goals for product 
211 1 Peter Amstutz
* Work with Product Owner to define overall release cycle 
212 1 Peter Amstutz
* Organize User Input and dialog with users for engineering team 
213 1 Peter Amstutz
* Contribute to backlog grooming 
214 1 Peter Amstutz
* Bring voice of customer into planning process 
215 1 Peter Amstutz
* Define user personas 
216 1 Peter Amstutz
* Coordinate user communication 
217 1 Peter Amstutz
* Develop technical marketing and sales materials 
218 1 Peter Amstutz
* Assist sales team in presenting product value proposition
219 1 Peter Amstutz
* Train sales in technical aspects of the product
220 1 Peter Amstutz
221 1 Peter Amstutz
h2. Definition of Done
222 1 Peter Amstutz
223 1 Peter Amstutz
An issue is resolved when:
224 1 Peter Amstutz
225 1 Peter Amstutz
* Code is written
226 1 Peter Amstutz
* Existing code is refactored if appropriate
227 1 Peter Amstutz
* Documentation is written/updated
228 1 Peter Amstutz
* Acceptance tests are satisfied
229 1 Peter Amstutz
* Code is merged in master
230 1 Peter Amstutz
* All Jenkins jobs pass (test, build packages, deploy to dev clusters)
231 1 Peter Amstutz
* Feature works on applicable dev clusters
232 1 Peter Amstutz
233 1 Peter Amstutz
h2. Standard Schedule
234 1 Peter Amstutz
235 1 Peter Amstutz
Sprints are two weeks long. They start and end on Wednesdays.
236 1 Peter Amstutz
237 1 Peter Amstutz
h3. Key meetings
238 1 Peter Amstutz
239 2 Peter Amstutz
Every day:
240 1 Peter Amstutz
241 1 Peter Amstutz
Daily Scrum (15 Minutes)
242 1 Peter Amstutz
Who: Development team, product owner. Silent observers welcome.
243 1 Peter Amstutz
* What did you do yesterday?
244 1 Peter Amstutz
* What will you do today?
245 1 Peter Amstutz
* What obstacles are in your way?
246 1 Peter Amstutz
247 1 Peter Amstutz
h4. Sprint review & kickoff (every 2 weeks on Wednesday):
248 1 Peter Amstutz
249 1 Peter Amstutz
Sprint Review (30 minutes)
250 1 Peter Amstutz
Who: Development team, product owner, stakeholders.
251 1 Peter Amstutz
* Demo of each feature built and relationship to stories
252 1 Peter Amstutz
* Product owner explains which backlog items are done
253 1 Peter Amstutz
* Development team demonstrates the work done, and answers questions about the sprint increment
254 1 Peter Amstutz
* Product owner discusses the backlog as it stands. Revise expected completion dates based on recent progress (if needed)
255 1 Peter Amstutz
* Review current product status in context of business goals
256 1 Peter Amstutz
257 1 Peter Amstutz
Sprint Retrospective (30 minutes)
258 1 Peter Amstutz
Who: Development team, product owner.
259 1 Peter Amstutz
* Review what processes worked well, and what didn't, in the sprint just finished
260 1 Peter Amstutz
* Propose and agree to changes to improve future sprints
261 1 Peter Amstutz
* Assign action items (meetings/tasks) to implement agreed-upon process improvements
262 1 Peter Amstutz
263 1 Peter Amstutz
Sprint Kick Off (1 hour)
264 1 Peter Amstutz
Who: Development team, product owner.
265 1 Peter Amstutz
* Add latest bugs or dependencies to sprint
266 1 Peter Amstutz
* Create tasks for each story
267 1 Peter Amstutz
* Assign a developer to each task
268 1 Peter Amstutz
* Assign an on-call engineer for that sprint who will triage customer support requests
269 1 Peter Amstutz
* Check that commitment level is realistic
270 1 Peter Amstutz
271 1 Peter Amstutz
h4. Planning (alternate Wednesdays mid-sprint)
272 1 Peter Amstutz
273 2 Peter Amstutz
Roadmap review (1 hour)
274 1 Peter Amstutz
Who: Development team, product owner, stakeholders.
275 2 Peter Amstutz
* Report high level status of epics
276 2 Peter Amstutz
* Prioritize epics
277 2 Peter Amstutz
* Define new epics
278 1 Peter Amstutz
279 2 Peter Amstutz
Sprint Planning (1-2 hours)
280 1 Peter Amstutz
Who: Development team, product owner.
281 2 Peter Amstutz
* Discuss and get engineering team consensus on feature design & implementation strategy for tasks on current and upcoming epics