2

A programmer keeps making cosmetic changes to the code while we have a strict deadline and the contract stipulate "no changes to the existing code". I am wondering where this "attitude" comes from: DevOps ? Agile ?

Changes performed:

  1. Replacing explicit variables with "var"

  2. Renaming short variable names with longer ones

  3. Refactoring code injections into MVC controller classes

  4. Adding design patterns (like command patttern) to existing code (with no functionality changes)

  5. Adding constructors with parameters to ViewModel classes (forgetting to add a non parameter one, so the post breaks...)

Hundred and hundred of changes after tests were made, and making merging way more complicated.

Is this Agile ?

7
  • 4
    Rewriting existing code to the command pattern is not a cosmetic change. It is a major refactoring. Commented Feb 27, 2020 at 13:24
  • 4
    The key point is "last minute". The changes should be rejected during code review Commented Feb 27, 2020 at 13:46
  • 1
    "I am wondering where this "attitude" comes from" - How about asking the programmer who is doing this? "Is this DevOps? Is this Agile?" - No, this has nothing to do with DevOps or Agile.
    – Jesper
    Commented Feb 27, 2020 at 15:19
  • No one can answer this other than the person making the changes.
    – 17 of 26
    Commented Feb 27, 2020 at 15:45
  • 1
    I would say good job, but don't merge the changes to master. Just wait until the next release. I am not sure what your source code control process is, but simple branching/pull request/merge should help.
    – Jon Raynor
    Commented Feb 27, 2020 at 19:02

4 Answers 4

8

These things sound like general good practice being over zealously applied.

But there is a easy answer

the contract stipulate "no changes to the existing code"

There's no arguing with a contract, revert the changes and discipline the programmer.

6
  • 3
    I upvoted this, but I have to express my unease with the "discipline" term. This seems like a team dynamics problem where the programmer is acting out. Architectural patterns and code standards are something the whole team should support, otherwise you get chaos. I like the idea of reverting the code because it sends a clear message that you can't run off on your own and violate the agreements. Past that, the right actions to align the programmer with the rest of the team seem very context-specific.
    – Daniel
    Commented Feb 27, 2020 at 16:01
  • you cant just revert though, someone has to do the hard management task and say "you just cant do this" its not a techincal discussion about whats best anymore
    – Ewan
    Commented Feb 27, 2020 at 16:34
  • Agreed. Sorry if that wasn't clear in my comment. Reverting sets the message that this won't be allowed. Then the team (including any team leadership like a manager) needs to address the underlying team dynamic problem. My unease with the term "discipline" is that disciplinary action is only one of many possible actions that may help the underlying situation. My point was not to say that you were wrong, but to expand on it.
    – Daniel
    Commented Feb 27, 2020 at 17:05
  • I don't see what the big fuss is all about. "Hey Joe, we had to revert your recent modifications in the master branch because the contract doesn't allow changes to the existing code. This clause in the contract is very important, and we must all make sure not to violate it. Let's put the changes into a separate branch and set up a team review to find out which can be merged into the master branch for future releases." Commented Feb 28, 2020 at 5:50
  • hey joe, one of your changes slipped through and now we are being sued. hey joe, we had to revert those changes and redo your work, now we missed the deadline and have to pay a penalty clause
    – Ewan
    Commented Feb 28, 2020 at 7:16
1

Some of these changes are sensible and will reduce technical debt, but there is a time for refactoring, and if you're under pressure right now, then this is not that time.

In the light of that, it sounds like they are just procrastinating. They are trying to look busy without having to tackle any of the difficult work that needs doing.

Ask them about the value of what they're doing. If you don't get a satisfactory answer, take it to your manager.

1

I am wondering where this "attitude" comes from

Remember what Developers do.

They create software solutions essentially from nothing. Out of thin air. We don't go and chop down some trees for raw materials before we start; we just sit down and start typing (well; some do).

That's almost "God-like" and Developers just love to "think" that they are God. Heck, that's why many of us got into this Business in the first place! (And yes; there have been [numerous] times when I could count myself among their egotistical number). Even better, when an organisation starts to "do" Agile or DevOps, Management actually start telling their Developers to play God - to "self-organise" and do everything "for themselves".

Is it any wonder this attitude ensues.

However ...

the contract stipulate "no changes to the existing code".

Game over.
Take away the Developer's keyboard (or access to update the code repository).

Replacing explicit variables with "var"

Whilst Type Inference is a great tool, in a truly Polymorphic implementation, this could actually break code. You might have had a very Good Reason for using a variable of a Base Type when the method creating that object creates a subclass thereof. Blindly changing it to the base class could cause chaos.

Renaming short variable names with longer ones

The compiler doesn't care what the variables are called.

Other Developers might care what the variables are called, if there is a convention across the codebase, which these changes are breaking.

Refactoring code injections into MVC controller classes

If Dependency Injection is used across the codebase and these changes are bringing this code into line with that convention, then OK. Maybe.

If this is introducing Dependency Injection to make the Developer "look good", then No.

Adding design patterns (like command pattern) to existing code (with no functionality changes)

Patterns are there as tools to help us do our job. If the existing code is already doing that job, then these changes are pointless.

I've been writing code for decades that other people now tell me are using Patterns.
Who knew?

Adding constructors with parameters to ViewModel classes (forgetting to add a non parameter one, so the post breaks) ...

... making merging way more complicated.

And now we get to the real issue.

This Developer is changing stuff and breaking it.

Just Say No. 

If they're breaking the Build, give them the Job of fixing it. Every time.

If they're breaking the Test suite, give them the Job of fixing it. Every time.

Either the Developer doesn't don't think they have enough real, value-adding work to do or they're choosing not to do what they're supposed to be doing, but that's another story altogether.
Instead, they are occupying themselves with this "busy work" which, while keeping their timesheet nicely topped up, is causing trouble for everybody else and, ultimately, will damage the project and, potentially, your organisation.

1

There are multiple problems.

One problem is making changes at the wrong time. Usually the sequence is that you create a release candidate, it is tested, unacceptable problems are fixed very carefully and retested, and when QA finds no unacceptable problems it is released.

Making 100s of changes during testing completely messes this up. That's something that should be just obvious to any developer. These changes have less than zero value. So that's a serious problem with the developer.

The second problem is that apparently you don't do code reviews. If these changes have no value to the company, then they should be rejected at code review time. It's one thing if I do five days work and add another four hours to address comments from a code review, that's to be expected (that's what you do code reviews for). It's another thing if someone does five days work and it is all rejected. That's five days wasted. And that's something that you can rightfully blame this developer for: Wasting five days time. Without code review and rejection it's much harder to complain.

On the other hand, I wonder what this contract term "no changes in existing code" comes from. There may be a reason for this. Someone else may be developing the "existing code" and when they release a new version, all your changes would be lost. They may want for good reason all your changes to be outside their own code. In that situation, yes, you shouldn't modify any existing code. Not only because the contract says so.

If "no changes in existing code" means "we don't want to pay for unnecessary work", then there are plenty of situations where modifying existing code is less work all in all. Which would be obvious to any developer and maybe not obvious at all to a non-developer. (On the other hand, this guys changes do not seem to fall into that category at all).

1
  • "no changes in existing code" comes from: do not improve the old existing/tested/in production code, we've been hired to add a new screen, at a very short date, and there are penalities I we finish late. The metaphor used was: do not repaint the old walls even if they need it, we've been hired to paint a new one. Commented Mar 2, 2020 at 13:04

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