Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update code insights planning process #4477
Update code insights planning process #4477
Changes from 1 commit
ae668f9
8c4a862
fb991d4
c6ca8b0
e7f84f2
b21ca7f
b7288e7
0608191
be993b1
eb98a10
c1b5ea2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I don't like this code freeze. I think it hasn't been particularly effective at stopping issues, and I think it leaves a lot of unresolvable ambiguity around what is
significant
. I also think it's a relatively low agency constraint that anyone in any form of leadership role should need to be an approver to merge code, as well as the more practical problem that leaving branches for many days in a very active repo can make merging unnecessarily complicated and risky.What were the original motivations behind this freeze?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coury-clark as I remember that that was introduced to avoid last-minute change before the release cut which we had pretty often (and I guess we still have it). I think this rule isn't bad but because we have been violating this rule for a long time already (as I remember the last 3 releases had these last-minute changes literally) we need to accept that this rule just doesn't work for us.
But it is also important to note that we usually violate this rule not because we want to ship another big thing but cause we usually find some problems on staging right before the release cut. We probably need to reconsider this rule. Like to be a truly code-freezed before the release cut I think we should end our active development 2-3 days before the code-freeze and then start the testing process actively to check things that we haven't covered by unit/integrations tests but with that system, we reduce our time for developing something for the next release. So I don't have a good solution to remove/replace this rule with something else but just mix feeling about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be okay replacing this with an alternative proposal, but I would otherwise keep this as the "default in lieu of a better solution." As the team grows I would expect our process to naturally mature (and as we grow we should have fewer moments where we have to choose between building in testing, since we should have a capacity to do both).
I consider "significant" to be roughly anything that modifies more than one directional flow or that is not just a copy change (copy changes often land close to release due to needing more stakeholders to review + catching things on dogfood, and I'm okay with that). You can also think of "significant" as "how many possible paths to test this are there?" and anything with more than 1-2 paths is likely significant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like us to specify where/how the testing goes. Do individual teammates add it to issues and does one of us review it? Do I/we include broader test plans earlier in this process? Either could work, curious what you were thinking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question (I hadn't thought about this as part of this PR). I would say both: The higher-level expectations could be documented as part of the scope agreement in the product RFC. Concrete testing for each sub-task (issue) I think should be included by the issue author (engineers, usually) when the issues are filed in the planning phase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this is practical and / or feasible. Not every project is going to involve the entire team for all the time, while some projects will require significantly more architectural overhead than others. We've had multiple workstreams throughout q2 - I don't really see how scaling up the team would reduce the need for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal ultimately is to avoid cognitive overload on individuals, so you can think of it as "not more than one project going on at the same time per person". It is very likely that as our team grows, we'll need to loosen this up to allow multiple groups of people working on different projects simultaneously. But at the same time, we prefer to operate together as a team rather than a working group where everyone is working on different things. At least right now, most big projects we do will involve backend and frontend together, and we're just 2+2 engineers. We also have only one designer and one PM. So I believe at the moment if we do multiple large projects at the same time, it will result in increased cognitive load for the team, which I want to avoid. And again, this may change in the future!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious - why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could technically also use labels for iterations and milestones for projects. We can't use both, because GitHub only allows one milestone per issue, but issues need to be possible to associate with an iteration AND a project at the same time – so we just need to align on one. Iterations are gonna be created more frequently, and they have a workflow of "closing" and due dates that labels don't have and are a bit more natural to display and edit in the new GitHub board (as it is a dedicated column). You also can't group by labels (or label "pattern") in the new boards, but you can group by milestone, which is used for the "All issues" view, which would not be possible if iterations were tracked by labels.