Project

General

Profile

Development process » History » Version 5

Ward Vandewege, 05/12/2021 08:21 PM

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