Project

General

Profile

Development process » History » Version 24

Tom Clegg, 05/23/2024 06:16 PM

1 1 Peter Amstutz
h1. Summary of Development Process
2
3
{{toc}}
4
5 22 Peter Amstutz
h1. Filing bugs and task scheduling
6 1 Peter Amstutz
7 20 Peter Amstutz
The Arvados project uses Redmine for organizing our work.  You are probably looking at the Redmine site right now.
8
9
h2. Where to create the ticket
10
11
Add issue using the "Issues" tab. 
12
13
Alternately, from the "Backlogs" interface, go to "Product backlog" and then "New story".
14
15
h2. Issue trackers
16
17
When filing an issue, use these guidelines to choose how it should be tracked:
18
19
|Bug|A flaw where the software does not behave as intended, or misleading/outdated documentation.|
20
|Feature|A well defined task taken to improve the software or documentation. This should be fully specified and actionable, otherwise use "Idea".|
21
|Support|An non-development action that needs to be performed to assist a user or customer, or an ops task to maintain internal or external systems.|
22
|Task|A task is a smaller unit of work attached to a feature or bug fix, most commonly used to communicate the state of code review on the task board.  *Note* this shows up in the "Tracker" drop-down for top level tickets but shouldn't be used unless you are filling in parent ticket.|
23
|Idea|An idea, concept, proposal or project to improve the software or documentation, which needs additional work to fully specify and/or broken down into concrete steps.|
24
25 21 Peter Amstutz
h2. Issue category
26
27
We use "category" to describe the part of the product most relevant to the bug or feature.  It is used to ensure that tickets are assigned to developers who are knowledgeable about that part of the product.
28
29 20 Peter Amstutz
h2. Writing good ticket descriptions
30
31 23 Brett Smith
When submitting a ticket, you should aim to outline:
32 20 Peter Amstutz
33 8 Tom Clegg
* Background / context
34 1 Peter Amstutz
* Current behavior
35 23 Brett Smith
* Desired fixes / improvements
36
37
If you have ideas about how the issue could be addressed, feel free to add:
38
39 8 Tom Clegg
* Proposed implementation
40 1 Peter Amstutz
* Exclusions / clarifications
41
* Open questions
42 23 Brett Smith
43
These can be added or refined as part of later grooming.
44 20 Peter Amstutz
45
h2. Ticket triage
46
47
At least once a week, the product manager will go through new tickets in "Product backlog".  (Maybe we should do this at a regular meeting?)
48
49 22 Peter Amstutz
If not done already, the ticket should be linked (using "Related to") to an "epic" or "request (tracking)" ticket (these are "idea" tickets).  If a ticket doesn't relate to any existing epics, it may represent a new project that needs to be added to "Epics".
50 20 Peter Amstutz
51
If a ticket is deemed sufficiently urgent/high priority, it will be scheduled for an upcoming sprint.
52 1 Peter Amstutz
53
Otherwise, following initial triage, the ticket should be added to the "Future" sprint.  This is the holding area for tickets that are not scheduled.  Because there are a very large number of tickets in this state, it is important to properly link tickets to epics or customer requests so they can be found again.
54 22 Peter Amstutz
55
h2. Ticket scheduling
56
57
If a ticket did not jump the line and be scheduled on a sprint, it will be pulled in during either the off week engineering meeting or during sprint preparation (the meeting on Tuesday the day before sprint turnover).  During these meeting we examine the current epics, customer priorities, and other internal priorities, look at tickets linked to those, and schedule them on an upcoming sprint.
58 8 Tom Clegg
59 1 Peter Amstutz
h1. Revision control
60
61
h2. Branches
62
63
* 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.
64
* Each story should be done in its own branch.
65
* Branch names are "####-story-summary" where #### is the redmine issue number followed by 3 or 4 words that summarize the story.
66
* Make your local branches track the main repository (@git push -u@)
67 9 Peter Amstutz
* Commit regularly, and push your branch to @git.arvados.org@ at the end of each day 
68
** Be paranoid, commits are cheap, pushing your commits to the remote repository is cheap, losing work is expensive
69 10 Peter Amstutz
** The preferred format of a commit message on a branch is like this (where 12345 should be replaced by the redmine issue number):
70
<pre>
71
12345: One line summary of changes in this commit
72
73
More detailed description of changes if relevant.
74
75
Arvados-DCO-1.1-Signed-off-by: Your Name <your.email@curii.com>
76
</pre>
77 1 Peter Amstutz
* Don't push uninvited changes to other developer's branches.
78
** 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.
79
80
h3. Merging
81
82 24 Tom Clegg
Branches should not be merged to main until they are ready (see [[#Ready to merge]] below).
83 1 Peter Amstutz
84
# @git remote -v@
85 6 Ward Vandewege
** Make sure your @origin@ is git.arvados.org, not github. *Don't push directly to the github main* branch -- let git.arvados.org decide whether it's OK to push to github.
86
# @git checkout main@
87 1 Peter Amstutz
# @git pull --ff-only@
88 6 Ward Vandewege
#* This ensures your main is up to date. Otherwise "git push" below might fail, and you'll be backtracking.
89 1 Peter Amstutz
# @git merge --no-ff branchname@
90
#* *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.
91
#* In your merge commit message, *include the relevant story/issue number* (either "@refs #1234@" or "@closes #1234@").
92 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>)
93 10 Peter Amstutz
#* The preferred format of a merge commit message is like this:
94
<pre>
95
Merge branch '12345-story-summary'
96
97
refs #12345
98
99
Arvados-DCO-1.1-Signed-off-by: Your Name <your.email@curii.com>
100
</pre>
101 1 Peter Amstutz
# @git push@
102 3 Peter Amstutz
# Look for Jenkins' build results at https://ci.arvados.org .
103 1 Peter Amstutz
104
h3. Rejected pushes
105
106 6 Ward Vandewege
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 main with consistent cross referencing with issue numbers.  These policies apply to the commits listed on "git rev-list --first-parent" when pushing to main, and not to commits on any other branches.
107 1 Peter Amstutz
108
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
109
110
  git commit --amend
111
112
to update the commit message on your last commit, if that is the offending one, or else you can use 
113
114
  git rebase --interactive
115
116
to rebase and fix up a commit message on an earlier commit.
117 6 Ward Vandewege
118 1 Peter Amstutz
h4. All merge commits to main must be from a feature branch into main
119 6 Ward Vandewege
120 1 Peter Amstutz
Merges that go the other way (from main to a feature branch) that get pushed to main as a result of a fast-forward push will be rejected.  In other words:  when merging to main, make sure to use --no-ff.
121 6 Ward Vandewege
122 1 Peter Amstutz
h4. Merges between local and remote main branches will be rejected
123 6 Ward Vandewege
124 1 Peter Amstutz
Merges between local and remote main branches (generally merges created by "git pull") will be rejected, in order to maintain a linear main history.  If this happens, you'll need to reset main to the remote head and then remerge or rebase.
125
126
h4. Proper merge message format
127 6 Ward Vandewege
128 1 Peter Amstutz
All merge commits to main must include the text "Merge branch 'featurebranch'" or they will be rejected.
129
130 10 Peter Amstutz
h4. All commits to main include an issue number or explicitly say "no issue #"
131 1 Peter Amstutz
132 6 Ward Vandewege
All commits to main (both merges and single parent commits) must
133 11 Peter Amstutz
include the text "refs #", "closes #", "fixes #", or "no issue #" or they will be
134 1 Peter Amstutz
rejected.
135
136
h4. Avoid broken commit messages
137
138
Your commit message matches
139
140
  /Please enter a commit message to explain why this merge is necessary/
141
142
h2. Commit logs
143
144 5 Ward Vandewege
See https://dev.arvados.org/projects/arvados/wiki/Coding_Standards
145 1 Peter Amstutz
146
h2. Code review process
147
148
Code review has high priority! Branches shouldn't sit around for days waiting for review/merge.
149
150
When your branch is ready for review:
151
# Create/update a review task on the story so it looks like this:
152
#* subject = "review {branch name}"
153
#* state = in progress
154
#* assignee is not null
155 12 Tom Clegg
# Comment on the issue page (not the review page), including
156
#* branch name
157
#* commit hash
158
#* link to Jenkins test run
159
#* if appropriate, a brief description of what's in the branch (may be omitted if it's already in the commit messages, or if it would just be a copy of the issue subject/description)
160 1 Peter Amstutz
# Ping your reviewer (during daily standup, via e-mail and/or via chat).
161
162
Doing a review:
163 13 Peter Amstutz
# Reviewers are usually assigned at sprint kickoff, but if you don't have a reviewer, ask for a volunteer in chat and/or at daily stand-up.
164 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.
165 6 Ward Vandewege
# The recommended process for reviewing diffs for a branch is @git diff main...branchname@.  The reviewer must make sure that their repository is up to date (or use @git diff origin/main...origin/branchname@). Note the 3 dots (not two)
166 16 Peter Amstutz
# The reviewer goes through the [[#Ready-to-merge]] checklist
167 14 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 by chat (or by some other means such as at daily standup).
168
#* In comments, it is helpful to indicate how strongly you feel how/important the comment is as "low", "medium", or "high"
169 7 Tom Clegg
#* low: nitpick not necessarily worth changing here if you don't feel like it, but I'm mentioning it to help improve habits
170
#* medium: suggestion/idea that you should at least acknowledge/respond to, even if we don't end up resolving it here
171 1 Peter Amstutz
#* high: we should make sure we both agree on how this is resolved before merging
172 7 Tom Clegg
# 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.
173 14 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 (indicating by posting a comment with "LGTM" for "Looks Good To Merge")
174
# When the comments are all low priority, someone might write something like "LGTM if you fix this one typo", this indicates that once the minor comments are handled (fixed or responded to) that the branch should be merged without another review cycle.
175 1 Peter Amstutz
# Once the branch is merged, move the "review" task to "resolved".
176
177
To list unmerged branches:
178 6 Ward Vandewege
* Yours: @git branch --no-merged main@
179
* Everyone: @git branch -a --no-merged main@
180 1 Peter Amstutz
181
h2. Ready to merge
182
183 19 Peter Amstutz
See also the "Ready-to-merge-checklist":https://dev.arvados.org/projects/arvados/wiki/Coding_Standards#Ready-to-merge-checklist
184
185 1 Peter Amstutz
When merging, both the developer and the reviewer should be convinced that:
186 6 Ward Vandewege
* Current/recent main is merged. (Otherwise, you can't predict what merge will do.)
187 1 Peter Amstutz
* The branch is pushed to git.arvados.org
188
* The code is suitably robust.
189
* The code is suitably readable.
190
* 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?
191
* 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
192
* 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.
193
* 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!)
194
* 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.)
195
* New/fixed behavior is tested. (Although sometimes we decide not to block on inadequate testing infrastructure... that sucks!)
196
* New/changed behavior is documented. Search the doc site for relevant keywords to help you find the right sections.
197
* Whitespace errors are not committed. (Tab characters, spaces at EOL, etc.)
198 18 Peter Amstutz
* Git commit messages are descriptive. If they aren't, this is your last chance to rebase/reword.
199
* Code meets other [[arvados:Coding Standards]]
200
* For GUI work: user interface elements meet accessibility guidelines on the coding standards page
201 1 Peter Amstutz
202
h2. Handling pull requests from github
203
204
_This is only for contributions by *external contributors*, i.e., people who don't have permission to write directly to arvados.org repositories._
205
206 6 Ward Vandewege
First make sure your main is up to date.
207 1 Peter Amstutz
208 6 Ward Vandewege
    git checkout main; git pull --ff-only
209 1 Peter Amstutz
210
*Option 1:* On the pull request page on github, click the "You can also merge branches on the command line" link to get instructions.
211
212
* Don't forget to run tests.
213
214
*Option 2:* (a bit shorter)
215
216 6 Ward Vandewege
Say we have "chapmanb  wants to merge 1 commit into arvados:main from chapmanb:branchname"
217 1 Peter Amstutz
* @git fetch https://github.com/chapmanb/arvados.git branchname:chapmanb-branchname@
218
* @git merge --no-ff chapmanb-branchname@
219
* Use the commit message: @Merge branch 'branchname' from github.com/chapmanb. No issue #@
220
(or @refs #1234@ if there is an issue#)
221 6 Ward Vandewege
* Confirm diff: @git diff origin/main main@
222 1 Peter Amstutz
* Run tests
223
* @git push@
224
225
h1. Non-fast-forward push
226
227
Please don't get into a situation where this is needed.
228
229 6 Ward Vandewege
# On dev box: @git push -f git@github.com:arvados/arvados proper_head_commit:main proper_head_commit:staging@
230
# On dev box: @git push -f git@git.arvados.org:arvados.git proper_head_commit:main@
231
# As gitsync@dev.arvados.org: @cd /scm/arvados; git fetch origin; git checkout main; git reset --hard origin/main@
232 1 Peter Amstutz
233
(At least that's what TC did on 2016-03-10. We'll see how it goes.)
234
235
h1. Working with external upstream projects
236
237
Development process summary (1-6 should follow the guidelines above)
238
239
# Each feature is developed in a git branch named @<issue_number>-<summary>@, for example @12521-web-app-config@
240
# Each feature has a "Review" task.  You can see the features and review tasks on the task board.
241
# 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
242
# The reviewer responds on the issue page with questions or comments
243
# When the branch is ready to merge, the reviewer will add a comment "Looks Good To Me" (LGTM) on the issue page
244
# Merge the feature into into the Arvados main branch
245
# Push the feature branch to github and make a pull request (PR) of the branch against the external project upstream
246
# Handle code review comments/change requests from the external project team
247 6 Ward Vandewege
# Once the external project merges the PR, merge external project upstream main back into the feature branch
248 1 Peter Amstutz
# Determine if external project upstream brings any unrelated changes that breaks things for us
249
# If necessary, make fixes, make a new PR, repeat until stable
250 6 Ward Vandewege
# Merge the feature branch (now up-to-date with external project upstream) into Arvados main
251 1 Peter Amstutz
252
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.
253
254 6 Ward Vandewege
This assumes using git merge commits and avoiding rebases, so we can easily perform merges back and forth between the three branches (Arvados main, feature, external project main).
255 1 Peter Amstutz
256
257
h1. Scrum
258
259
h2. References
260
261
These books give us a reference point and vocabulary. 
262
263
* _Essential Scrum: A Practical Guide to the Most Popular Agile Process_ by Kenneth Rubin
264
* _User Stories Applied: For Agile Software Development_ by Mike Cohen
265
266
h2. Roles
267
268
269
h3. Product Owner
270
271
* Decide what goes on the backlog
272
* Decide backlog priorities
273
* Work with stakeholders to understand their requirements and priorities
274
* Encode stakeholder requirements/expectations as user stories
275
* Lead sprint planning meetings 
276
* Lead release planning meetings 
277
* Lead product planning meetings 
278
* Lead Sprint Kick-off Meetings
279
* Lead Sprint Review Meetings
280
* Decide on overall release schedule 
281
282
h3. Scrum Master
283
284
* Lead Daily Scrum Meeting
285
* Help to eliminate road blocks 
286
* Lead Sprint Retrospective Meetings
287
* Organize Sprint Schedule 
288
* Help team organize and stay on track with Scrum process
289
* Teach new engineers how Scrum works
290
291
h3. Top stakeholders
292
293
* Conduct market research 
294
* Synthesize market research into user stories 
295
* Work with Product Owner to prioritize stories
296
* Define overall business goals for product 
297
* Work with Product Owner to define overall release cycle 
298
* Organize User Input and dialog with users for engineering team 
299
* Contribute to backlog grooming 
300
* Bring voice of customer into planning process 
301
* Define user personas 
302
* Coordinate user communication 
303
* Develop technical marketing and sales materials 
304
* Assist sales team in presenting product value proposition
305
* Train sales in technical aspects of the product
306
307
h2. Definition of Done
308
309
An issue is resolved when:
310
311
* Code is written
312
* Existing code is refactored if appropriate
313
* Documentation is written/updated
314
* Acceptance tests are satisfied
315 6 Ward Vandewege
* Code is merged in main
316 1 Peter Amstutz
* All Jenkins jobs pass (test, build packages, deploy to dev clusters)
317
* Feature works on applicable dev clusters
318
319
h2. Standard Schedule
320
321
Sprints are two weeks long. They start and end on Wednesdays.
322
323
h3. Key meetings
324
325 2 Peter Amstutz
Every day:
326 1 Peter Amstutz
327
Daily Scrum (15 Minutes)
328
Who: Development team, product owner. Silent observers welcome.
329
* What did you do yesterday?
330
* What will you do today?
331
* What obstacles are in your way?
332
333
h4. Sprint review & kickoff (every 2 weeks on Wednesday):
334
335
Sprint Review (30 minutes)
336
Who: Development team, product owner, stakeholders.
337
* Demo of each feature built and relationship to stories
338
* Product owner explains which backlog items are done
339
* Development team demonstrates the work done, and answers questions about the sprint increment
340
* Product owner discusses the backlog as it stands. Revise expected completion dates based on recent progress (if needed)
341
* Review current product status in context of business goals
342
343
Sprint Retrospective (30 minutes)
344
Who: Development team, product owner.
345
* Review what processes worked well, and what didn't, in the sprint just finished
346
* Propose and agree to changes to improve future sprints
347
* Assign action items (meetings/tasks) to implement agreed-upon process improvements
348
349
Sprint Kick Off (1 hour)
350
Who: Development team, product owner.
351
* Add latest bugs or dependencies to sprint
352
* Create tasks for each story
353
* Assign a developer to each task
354
* Assign an on-call engineer for that sprint who will triage customer support requests
355
* Check that commitment level is realistic
356
357
h4. Planning (alternate Wednesdays mid-sprint)
358
359 2 Peter Amstutz
Roadmap review (1 hour)
360 1 Peter Amstutz
Who: Development team, product owner, stakeholders.
361 2 Peter Amstutz
* Report high level status of epics
362
* Prioritize epics
363
* Define new epics
364 1 Peter Amstutz
365 2 Peter Amstutz
Sprint Planning (1-2 hours)
366 1 Peter Amstutz
Who: Development team, product owner.
367 2 Peter Amstutz
* Discuss and get engineering team consensus on feature design & implementation strategy for tasks on current and upcoming epics