A developer will occasionally send me a pull-request that, whilst often technically excellent, has new code in it that is ready, waiting for some sort of future requirement they have convinced themselves will be needed.

A simplified example may be adding a birthday field to the users table. The developer will add the field, make the user form cope with the new field, but then also take it upon themselves to add a birthday_email_sent_at field, because they believe that surely we will want to send birthday emails to the user sometime in the future.

There are a number of issues at play here:

1. YAGNI (you ain’t gonna need it) — A popular term that describes this phenomenon. Building more than you need.
2. Small PRs. The more “features” tacked onto the PR the more unwieldy it gets. My philosophy of late is to try and get one commit into a PR. A small focussed, atomic change that can be easily reviewed. Sure at times this can be multiple commits if the change needs to be done in one merge. Readability is key here.
3. Testing — adding something “we may need in the future” puts strain on the testing systems. Not to mention adds time to the entire CI flow.
4. Shows bad design. If something is being added now, out of scope, because “its easier to add it now” show that the classes in question are too large and not focussed enough.
5. Lack of communication. If a developer honestly believes that an additional change over and above the brief is required then that should be communicated well and truly before a single line of code is written, with agreement from all stake holders. And even then it will probably become a new card/brief and another PR anyway.

A better way?

I recommend the following approach:

1. Make sure the cards/tasks assigned to developers are clear and unambiguous. It starts here. Don’t give your developers any breathing room for misinterpretation.
2. Make sure developers are comfortable asking you for clarity on the feature/request.
3. Reject PRs (or kindly ask that code is removed) that adds functionality or future proofing not requested on the card. This is more a training technique than anything. The more you push back the more the developer will come to understand that only the requirements of the card will be accepted. A good comment may look like “Can these lines be removed please, as not in the requirements. They look like an interesting add — can we discuss for a later stage?”
4. Use the rejected PR as an opportunity to also ask the developer how they could design the classes to be easily changeable should a change be required tomorrow.
5. Do all the above in a dignified manner. In most of these situations the developer is acting out of genuine care for the product. Ensure the developer isn’t left feeling like their mistaken or contribution was a waste.

Doesn’t this harm initiative?

No. Initiative is welcomed and encouraged. It’s actually a great trait to see in developers. Through communication and discussion, initiatives can be addressed and actioned prior to a line of code being written.

To get back to the birthday field example from above — a developer, using initiative, could have asked prior to development — “With this update, should I add a birthday_email_sent_at field? We’re going to be sending emails to customers at some point right?”

This starts a discussion, which is significantly cheaper than coding something for tomorrow that may not be needed.