0

I'm working in a team of about 15 people and we're currently in the process of switching from svn to git and establishing a new workflow with git.
I decided to use gitflow as our new workflow because it can solve some issues we had with our not-really-existent workflow using svn where everyone just put his/her code into the repository, sometimes without really thinking too much about it. That often lead to broken code and mad devs because they couldn't continue their work temporarily.
With gitflow I wanted to try to force the people to change their behaviour step-by-step but apart from the issues with swapping the svn mindset by the git mindset (some devs didn't work with git in the past or even at all), we can't really agree on how to use this workflow efficiently.

There are many commits that just contain some reformatting of code or some tiny fixes (like typos) and these commits just get committed into our develop branch right away without creating an extra feature branch for it.
Then we have some time-related issues. Who is responsible for approving merge requests?
We can't afford to put the load on one person as the "master" for merge requests and it doesn't make sense in my opinion.
So instead we initially agreed to just peer review the code and then the creator of the merge request does the actual merge but this approach has the flaw that not everyone reviews every request and so it takes quite long until x-many people reviewed the code. Where to put the threshold?

I guess in short we have some issues in defining the rules when someone has to create a feature branch and how the peer review process should be organized in order to not slow down too much.

Unfortunately I cannot find any resources on how other teams solved this.
I'm even not quite sure if I'm missing some very important points here in working with this workflow.
I also have to add that we don't have automated tests (yet) as the former development team didn't do this for years. So this is definitely another thing that might be a major obstacle for us.

Finally I decided to get here and ask for your help.
Do you have any advice that I could use to solve (some of) these issues we have?

3
  • articles / resources recommendations are off-topic per help center
    – gnat
    Jul 31, 2017 at 16:09
  • i would rather count it as a mix of software development methods and practices and quality assurance and testing
    – TorbenJ
    Jul 31, 2017 at 16:18
  • 1
    What about forcing developers to create pull requests for everything, and allow your tooling to automatic approve pull requests upon successfull build. I'm not familiar with gitflow (git/tfs user here) but that would take the least amount of resources while still guaranteeing a minimal quality (the code compiles)
    – Batavia
    Jul 31, 2017 at 16:23

3 Answers 3

3

We set our threshold at having two peer reviews on a pull request before merging. That means for every one you submit, on average you are reviewing two. That is not too much of a burden.

On small, typo/reformatting fixes, we generally only ask for one peer review. Yes, we occasionally catch things on these reviews, because people think they are so simple they don't need to be as careful. On large, sweeping architectural changes, we will ask for more people to review, but for day to day work, two is sufficient.

It doesn't have to slow you down. You can continue your work while you wait for peer reviews. I quite frequently pull branches back and forth between a colleague before we submit it for final review.

Mostly, people just need to get used to the benefits. Once your reviews, and hopefully later a CI server, start catching errors, people will be more willing to do them.

2

Don't worry about the small commits and permissions. What you want to work towards now is people working on tickets rather than random things.

Get a ticket system running and ask people to check all work in in a feature branch with a name including the ticket number and ticket title. Tell them its so you can link the systems up (which you can)

Pull people up when they work on things without tickets, rather than the odd missing feature branch. As this is the root cause of the chaos. Not the check-in strategy.

I would not recommend pull requests or code reviews. But 15 is quite a bit team. I would suggest you split it into 3 teams of 5

DO concentrate on automated testing as yor next priority. It really is essential

9
  • 2
    How can you not recommend code reviews? Obviously projects have limited time and resources, but they should be performed as much as possible. I have never not learned something from a good code review. Jul 31, 2017 at 16:43
  • because code reviews are simply a barrier to progress. smaller teams and better communication will achieve far more
    – Ewan
    Jul 31, 2017 at 16:45
  • I agree with your recommended team size but that is not mutually exclusive to performing code reviews. On the contrary the communication benefits of small teams will improve the yield of code reviews. Jul 31, 2017 at 16:48
  • If you find that code reviews are inhibiting progress because people are nitpicking too much, it means you need a team coding standard and that you should enforce it with automatic formatting and refactoring as much as possible Jul 31, 2017 at 16:51
  • 1
    no-one is 'enforcing' it, its a gentle process of encouraging change. Why don't you put up an answer recommending code reviews, coding standard documents and various gate keepers?
    – Ewan
    Jul 31, 2017 at 17:04
1

Why are you switching to git? Switching to git may not fix the issues you are having, and in fact may just create more. SVN is a nice modern VCS that should be plenty workable. Before switching to git, you may want to look more deeply into the problems you're having now, and see if they can be solved in SVN; if so, that makes it easier for all involved. Changing your VCS without changing your process won't solve any of your problems.

That said, some answers to specific points in your question are below. Note that these are independent of the VCS you are using. The tl;dr answer is you just need to experiment and see what works best for your team in your situation. Be willing to try something out for a couple weeks / months, and modify it if it's not producing the desired effects.

Code reviews / pull requests are a great way to prevent broken code and mad developers - if they are done. You want to be careful not to have the review be too time consuming, or else people won't do them (in a timely fashion). You also want to be sure that developers are not idly waiting around for reviewers to look at and approve the code; they need something else to work on in the meantime. Make sure your backlog isn't empty.

Quick means not only the number of reviews, but also the size. Two reviewers per pull request is a good starting point, but in a team of 15 you might be able to have more. This ties in to the size of the reviews, as well; if they take half a day, people won't want to do them and it'll be difficult to even get 2 reviewers. If they're quick, 15 minutes or so, you may find a lot more reviewers looking at them.

You also have to ensure that most of your pull requests are of a manageable size - I've found that substantial changes in more than 4 or 5 files can be a bit overwhelming to do in between tasks, as that requires a lot more concentration. But again, you'll need to experiment to see what works best for your team. Of course, a large review every now and then is usually unavoidable, but don't make it the norm.

On the flip side, tiny reviews consume a lot of overhead. You can either submit many tiny changes as one pull request, or waive the PR requirement for trivial changes (like typos or minor bug fixes) and just make the changes on the main branch. You'll have to experiment and see what works for your team.

Use automation to take care of code formatting, so you don't even need to worry about those pull requests. This has the benefit of keeping all the code in one style, as well.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.