4

We had a situation in which a senior developer decided to change some code thinking it would be for the best, which it is, but we still had some problems during that period. Some functionalities broke (no unit tests), but the changes did make sense.

What I'm looking for now is to avoid this happening again and also improve developer interaction with each other. I'm thinking some kind of policy or process to achieve this. I wanted to ask what you have experience doing and see if we can apply it for our team. Currently, what I have is peer reviews, and also before implementing something new, to have a meeting with the seniors (at least 2 seniors) on how to implement that functionality as having more set of eyes (and brain) can help something to be implemented better.

  • 3
    This fails to define the problem. senior developers change code for the better all the time. Having "some problems" is the cost of being alive. How does a plan to improve "developer interaction" relate to keeping functionalities from breaking? Can't you tell us how a poor interaction caused a problem? – candied_orange Oct 9 '16 at 8:33
  • @CandiedOrange: If there were some other set of eyes before he made the changes then maybe the problem wouldn't be that large. Or if his changes were reviewed by other senior devs. So his interaction with us during this instance wasn't good, if it was then maybe we could've lessened the impact – g_b Oct 9 '16 at 8:50
  • 2
    Add unit tests. End of the problem. 4 eyes can still fail at code review. Also 8, 10, 12... But tests are good at doing any sort of repetitive validation tasks. Your are pointing to your devs as the source of the problem... Bad idea – Laiv Oct 9 '16 at 9:32
  • What is your workflow process? – Dan-Cook Oct 9 '16 at 10:20
  • 6
    To be clear, no unit tests broke? Or no unit tests exist? – svidgen Oct 9 '16 at 15:19
3

First you need good unit tests.

I say first because if you focus on 'developer interactions' before doing this you will be introducing a lot of process and overhead that supports doing the wrong thing or at the very least will be unnecessary when you then have automated tests. Also resources and enthusiasm for change are limited. Start changing the worst thing first. Developer interactions are critical but unit tests are even more criticial.

So use your time to take the first step of writing unit tests. tomorrow. If folks don't know how to or the framework to use etc, make training and education tomorrow's activity and start writing the tests next week. Make sure that folks know that this will be hard, will have a steep learning curve, and even when up to speed will often mean taking more than twice as long as just writing tests. This is an investment. You can build a house without a foundation more quickly but it will then have issues during use (based on the expected 100 year timeframe it should last). If a framework or getting started seems intimidating write a piece of code that calls one of your functions (methods, etc.) passing in appropriate parameters and checks the return value to see if it is what is expected. boom. you have a unit test. repeat.

Start with requiring unit tests for new functionality. Over time the coverage will grow. Use a tool to measure code coverage over time so you can see this.

Champion the idea that unit tests are the only practical way to ensure that new changes don't break exiting functionality for todays development world. Relying on manual testing changes the code change cycle from minutes to days.

If you are doing Agile development talk about the Unit, Functional, Stress and Exploratory testing that you should consider. Unit testing is just one of these quadrants. It's the one area that I consider non-optional and unit tests should be written in concert with the code. Ideally ahead, step by step, but in practice you may in some cases write the code then the test and in others the test then the code. On a small scale the details of which you do first don't matter as much (there are differences though) as making sure that you end up with code and tests that support it. If you do write the tests first though you will generally find that the code ends up being better and more testable!.

How will these new processes/activities improve developer interaction?

Having a solid reliable comprehensive set of unit tests will help people feel ok about changing code, ok about quick fixes or bug fixes. I know Joe won't break Mary's code because the good unit tests are there to tell me if I do. Otherwise days are filled with various developers breaking various other developers code, not realizing it and introducing bugs constantly. Been there, done that, not fun. It's hard to have a fun, relaxed friendly environment when all this is going on. On the other hand an environment where we are all protected from this by tests can be more relaxed and less stressful in my experience.

I have worked at several companies at different points on the spectrum of having good test suites and have seen the above play out at them. Don't rely on a second set of eyes for breaking existing functionality when automated tests can do it for you. Retain the second of set of eyes for higher level activities surrounding well-crafted code that computers can't do well.
yet.

One more point - also use linters for your code to check format for html, css, ruby, javascript, python, etc. This is another way to help stop code reviews becoming personal and heated (thus degrading those developer interactions you are asking about). You make decisions on code standards and capture those in your linter configuration files. This also lets you talk about higher level stuff and not be arguing on indent spaces, etc.

p.s. this is all new to me. When I wrote code 20 years ago, manual testing was the only option.

  • I think it is a little naive to say start adding unit tests from tomorrow will fix their problems, but I do agree that they are an investment overall and should be part of development process. My main concern would be explaining the added developer days to a client (if there is one in this case). Perhaps if they were to instead, focus on adding coverage to all newly added features (bringing in TDD), and to any existing functionality that they are affecting directly or indirectly? They can still continue to add past coverage without affecting client expectations as much. Thoughts? – Dan-Cook Oct 9 '16 at 13:16
  • 1
    Added language indicating that this will be the first step not that it will just "fix their problems". Definite YES to doing it first for only new functionality. Will update with that. Thanks! – Michael Durrant Oct 9 '16 at 13:21
3

The senior-junior thing is so 1990s. It is ok if more experienced engineers have a heavier weigh in into issues if they can back it up and get the whole team to agree on a solution. However, changing things out of the blue without the team's buy in is not ok. Feeling like they are somehow more superior to the other developers is not ok.

It is good you all are doing peer reviews but again just make sure you truly are 'peers' in that and it is not slanted towards seniors dictating what is happening. If you ever feel like it would negatively effect you to discuss issues in an open manner, it is probably not a peer review.

One of the things I like for situations like this is implementing retrospectives. It's just a meeting where the individual team members can bring up issues in front of the entire group and openly discuss them. Agree or disagree openly if something is truly a problem and come up with a way forward to fix the issues. Some teams are able to openly discuss problems on an ongoing basis, other teams need a more formalized meeting like a 'retro' to slowly change the culture of development from a command/control type to a more open, equal environment where creativity can really kick in.

2

I believe this is a classic XY question. You are asking for how to achieve something ("developer interaction"), but the problem you actually have seem to call for a different solution.

You have a developer which make a change which you consider an improvement, but it caues an unexpected regression which is not caught by unittests.

Reviews are always a good idea, but you state that you already have peer reviews and meetings with several senior developers discussing changes. So it seems the problem happened despite developer interaction rather than due to lack of it.

There are a few possible explanation:

  • the developer was reckless, and should immediately have realized the error. And the peer review was done sloppily.

  • your code is brittle or hard to reason about which means even changes which are carefully examined might still lead to unexpected errors.

Furthermore the issue was not caught by a unittest. There are a few possible explanations:

  • You do not routinely write unittest to verify changes and prevent against regressions.
  • You do write unittests, but they are badly written and does not actually test anything.
  • It is really hard to test your code, or the code is really complex and hard to reason about, so bugs are often not caught despite unit test.

You have too look into what is the cause of the problem before you try to implement a solution.

1

There have been some good points about upping the communication and including unit tests, what I feel is missing from the discussion is how you go about implementing said changes after your team is in agreement / has awareness. It sounds like you either do not have an established workflow process, or your current one is no longer suitable.

You say that you have peer review, yet these changes broke existing processes which would seem to also indicate a lack of regression tests / or lacking suitable ones (despite also lacking of unit tests).

Some suggestions:

  1. Do consider creating an end to end test plan which can be executed by a peer/QA to test your functionality whenever these changes are introduced. This will at least help cover your lack of unit tests (obviously, it cannot replace them altogether).

  2. Consider revising / updating your existing workflow to a more appropriate one, something like Git feature branching could suit your needs (https://www.atlassian.com/git/tutorials/comparing-workflows/feature-branch-workflow). Using this in an environment such as GitLab, your team will have full visibility of changes, be able to contribute to discussions, and checkout changes in a separate testing environment, etc.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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