Open ellerychan opened 9 years ago
Didn't realize this issue already existed; opened Issue #92 and now marked it as a duplicate of this one. Here is the text from Issue #92:
The Master Server team has started a Process document. Currently this states the proposed process for managing cross-repo issues ingested during the migration, and a proposal on how to work with labels to track various issues.
Need to review and discuss if we want to adopt this across all the teams, and if not, where we can meet in the middle to have a single process across all the teams.
Added and Issue Process section to the Process page as a starting point. Needs refinement.
Items to discuss:
state00-submitted
state and think it should be removed.in work
state. We have a design required
label anyway to mark issues that need some up-front thought.How much process do we want/need? Do we feel we need reviews before an issue branch gets merged to the hsdc2016 branch? Do we feel we need regression testing before an issue branch gets merged to the hsdc2016 branch?
How much process do we want/need? Do we feel we need reviews before an issue branch gets merged to the hsdc2016 branch?
I think it has to be kept minimal for this type of volunteer effort. We don't want progress to get held up by waiting for a review. We may need to live dangerously. We can rely on peer pressure to keep the quality up to an acceptable level.
Do we feel we need regression testing before an issue branch gets merged to the hsdc2016 branch?
It would be great for modules to include some unit tests, but that could be an effort unto itself, and we may not have the luxury. (Python has a convenient unittest framework.)
Agreed. In that case I propose we go back to the In-work/Resolved states that we had on Google Code since that was working for us. We definitely need more than just Open/Closed, so I'm thinking we need the following:
Process document needs to be updated to capture this pending approval.
Sounds reasonable to me. I'll put it to the test here shortly. brata-hsdc/brata.masterserver#11
We need one more state. If we're using the absence of a state to indicate newly-submitted issues, we also need to keep track of previously-submitted issues that are not being placed in-work; otherwise, we'll continue to see them over and over while they're not yet in-work.
I think if we're down to these few states, then we can repurpose the "wont fix" priority to be the "hold" state because we don't need to track in which state an issue has been put on hold.
We now have:
I changed the default branch for all four repositories to hsdc2016 and updated the Process page to add some info for Pull Requests. I haven't tried a Pull Request myself yet, so the entire area is covered with a TODO still.
@kjones27 I saw your e-mail. I agree discussing here would be a better idea so everyone sees the answers and the discussion is maintained here and not lost after 90 days.
Please feel free to update the wiki page to improve/clarify for the next reader.
- Under “Working an Issue” section, step 9 says to commit and push the changes as you work. Just making sure this means you’re cool with us pushing changes that aren’t fully completed? I like this, as working a major change all locally always concerns me (computers/hard drives fail). But, I just wanted to make sure we were on the same page. :)
Let's confirm with @ellerychan but I believe this is correct. If we're working on our own dedicated issue branches, then we can commit to our heart's content and not worry about impacting others, so I vote we do push frequently (e.g. nightly) to mitigate the risk of hard drive failures, data loss, and the infamous taking talents elsewhere argument.
- “Working an Issue”, step 11 says to be sure you test changes sufficiently. Are we using any kind of test framework, or is this informal testing? I’m assuming the latter, since this is a volunteer effort, and we have to keep it somewhat mean and lean. But, let me know if I’m wrong. Also, if it’s the former, are there any existing regression tests or anything?
Nothing defined yet. Agreed to keep it light since this is a volunteer effort; however, if anyone wants to investigate and begin something and write a couple unit tests, then maybe we can request that the next person touching to code maintain them.
I think the idea is that we have enough of it working for next year's contest, but if we do get to a good point early on, we will have more time to investigate things like this, if for no other reason than to expand our own personal knowledge of how to develop code in Python/Django.
Personally, I'm not a fan of TDD/TFD up-front for new development; I think it's something that should start after something like the initial release due to all the major scrapping and rewriting that goes on during initial development, so I think now would be a good time to start adding in unit tests for the station code and perhaps switching to TFD/TDD there, but I'd want to wait a few months till the dust settles on the Master Server rewrite.
- “Completing an Issue”, step 10: How are we reviewing changes and providing comments?
We just tried one for Pull Request #12 last night for @ellerychan's changes in Issue #11. I think the interface works pretty well, though I'm not thrilled about one e-mail for every comment posted. @ellerychan probably filtered mine to go to his Junk folder by now.
- At what point do we merge our branch back into the main branch? I may have missed that part.
If I understand the process correctly, most of us work on our issue branch, address pull request comments, and then leave it alone.
I'll let @ellerychan confirm, but I think the proposed process involves one POC from each team merging the pull requests to the hsdc2016 branch and testing on the hardware to make sure the changes didn't break anything.
We either merge to master when we're completely done next year, or we merge to master when we have sufficient development merged on the hsdc2016 branch and we're ready for the competition, and then we do it again as we incorporate additional fixes. I'm still unclear on this.
If we're working on our own dedicated issue branches, then we can commit to our heart's content and not worry about impacting others
I agree. We only really need to collaborate via a Pull Request when merging code into a shared branch, and I would think we may have some collaborative efforts (like two people working on a specific feature, for example) where they would make a feature branch, then both make their own personal branches off of that. They could do Pull Requests to merge their branch periodically into the feature branch, and would keep their branch updated by pulling other people's changes from the feature branch. Eventually they would deliver their feature to the main branch (hdsc2016). If the collaborators feel they can coordinate well, another approach would be for both of them to pull and push between their own repositories and the feature branch directly. (So in their own repositories, they would be working on a clone of the feature branch directly.)
The first approach lets each contributor publish his/her changes before merging them into the shared branch. The Pull Request is the notification mechanism. However, there is no Pull Request before the merge in the second approach, so there would be no mechanism to see and comment on the changes before they were merged into the feature branch.
“Completing an Issue”, step 10: How are we reviewing changes and providing comments?
I think the still undefined part of this is, how do we know when we are done? There is no guideline for declaring something ready and closing it out. I don't know if there should be a hard rule. Teams will probably work differently depending on their mix of personalities.
Some options:
If I understand the process correctly, most of us work on our issue branch, address pull request comments, and then leave it alone.
Not sure about the "then leave it alone" part. I thought it would work best for the Pull Requester to merge his/her changes into the target branch unless someone is clearly the repo "owner". I think in many cases, no one on a team is going to be intimately familiar with all the code in the repo. (Volunteer effort.) So the Pull Requester will have the best knowledge of the code being merged, and what it is likely to affect. The Pull Requester would do the merge and take a significant part of the responsibility of making sure everything still works.
Caveat: I am about the farthest from being a process guru, so I feel a little funny making recommendations about process. I think the ideal to strive for is helpful, yet unobtrusive procedures. Another phrase to keep in mind: "low-hanging fruit."
Programming is a complex task (we're not talking about laying out 100 report screens in Visual Basic), and anything that distracts or causes you to lose focus kills your productivity.
Fine with me. So if I work on a feature, I'm responsible for merging it back in when I feel it's ready after sufficient time has elapsed on my open Pull Request.
Like I said, it may not be right for every team, but I'd vote for that approach for this team.
I updated the Quick Start. Search for "[Proposed]" to see the edits.
I remember @ellerychan asking what to do about pull requests over N commits and being able to review the completed set rather than reviewing one commit at a time.
Project Hydra appears to require squashing commits in their process for this reason. I've been hesitant about that because it seems to rewrite history from what I've read, but maybe it's the lesser of two evils or perhaps it's not really as bad as I think.
Craft a procedure that allows us to reliably track issues and get them resolved, while keeping the burden of issue management minimal.