DEV Community

Discussion on: Performing a PR Review

Collapse
 
martyonthefly profile image
Sylvain Marty

Thanks for your article !

I'm a developer from 2 years now and I still need to deal with the "Your code is bad but it ain't personal" and I'm ready to learn from my mistakes and from my team mates though.

But when the manager say "I need something which works in four hours", I'd love that this information would be taken in consideration when the lead developer perform the PR review...

How do you deal with this kind of case ?

PS: english is not my native language so sorry if it's not clear ! Feel free to correct me :)

Collapse
 
subbramanil profile image
Subbu Lakshmanan

If my understanding is correct, You are referring to a situation where the manager wanted to get something done in limited time(4 hours in your case) and you ended up delivering the feature that works but may not be up to the standard(coding style/technicality/implementation details).

I agree that the situation is more common and the technical term is "Technical debt", where the delivery of the feature prioritized over the code quality/implementation details.

At the basic, I would suggest adding a comment on PR that why particular decision was taken (timing constraints, limitations, etc.,) and possibly a "To-Do" in the code to mark the implementation is not completed yet.

However I'd tread carefully on what is considered as "Technical debt".

If my answer satisfies your question, I would recommend reading this.

Collapse
 
martyonthefly profile image
Sylvain Marty

Thanks for your answer ! I think I still got some difficulties with the technical debt when a feature is urgent because end users are in pain with the actuel system (or without system at all).

The article you gave to me is great !

I think we, managers and code reviewer, must care about technical debt only when the end users aren't in pain with the current system and when they don't need a quick release to solve a job issue. Finally, if we choose the dirty approach to solve a problem quickly and to make the users happy, it's only us, devs, which gonna have pain in the future. I think we must find the middle ground :)

Again, thanks for your answer, I learned a lot ! :)

Thread Thread
 
subbramanil profile image
Subbu Lakshmanan

Yes You are correct. To quote from a book that I was reading earlier,

“It is better to ship something and accept some prudent technical debt than it is to be late for the sake of gilding a working solution. That said, every project is unique in its tolerance for timeliness versus completeness.”

Excerpt From: Gary McLean Hall. “Adaptive Code: Agile coding with design patterns and SOLID principles, Second Edition

Thread Thread
 
martyonthefly profile image
Sylvain Marty • Edited

Nice !

It is funny that I met today at my job the exact situation described in your exerpt. We build a new system from nowhere which was badly integrated in our current system. In one hour, we could make the new system work permanently with the previous one without any conflicts (the dirty way). Or, we could rebuild all the system from scratch with new specs and new hands (the cleanest way). But we had users which have waited a lot of time for this new system. From this, the decision has been made : we are assuming the technical debt. To solve the debate, a teammate asked this question : "Why the end users have to pay for ours bullshits ?". And for the first time, I knew that the cleanest way is only about us... :)

I will look after this book, thanks for sharing !

Thread Thread
 
dijitalmunky profile image
Chris Roe

Lots of good things in this thread so far! I can't offer a guaranteed solution of course. When I get into this situation, I have always found that communication is key. I would also suggest that face-to-face or at least voice-to-voice communication between the 3 parties is key. What I see likely has happened here is that your manager has inadvertently given you one direction ("I need something working in 4 hours"), and your team lead a different direction (something like "It is your responsibility to make sure the code is solid and maintainable"). Each request in and of itself is reasonable. However, taken together, they are sort of at odds with each other. I am guessing that neither of them has realized this. So a quick conversation with both of them will likely help.

Another thing that I have found has helped hugely with these situations is iterative development. Sure, get something done and working in the 4 hours required by your manager. Let your team lead know that this is your first goal and is your priority and that it needs to go through. Then iterate once or twice more to fix bugs and clean up the code styles.

Collapse
 
dijitalmunky profile image
Chris Roe

The whole getting past "Your code is bad but it ain't personal" is quite hard to get past, but hang in there you will get it! It took me the better part of 20 years myself!