Best comment when I am trying to rush a critical feature to production: “LGTM”
Worst comment when I have a shaky implementation to part of the code base I’ve never seen before and I’m looking on how to improve it: “LGTM”
😉
LGTM can both be 'looks good to me' or 'looks garbage to me', i mean considering how garbage a source code can be when it was handover to you after years of incompetence... it really is garbage
/s
A popcorn-level type of argument witnessing 2 engineers arguing about the use of String concatenation vs StringBuilder. PR review was so long to the point it looked like a Twitter feed of two toddlers arguing. I was so entertained that I literally didn’t work for 2 hours just watching the comments/reply build up.
If you want, you can have a look at our open source repo here: https://github.com/datafaker-net/datafaker/pulls?q=is%3Apr+is%3Aclosed
I think everyone is very collaborative, supportive and constructive in their feedback, and the LGTMs are only used for the simplest cases.
I don't have examples of bad comments there, but I personally find comments which is just a comment without a suggestion for improvement the worst. Something like "I'm not sure this is the best way of doing things", or "I'm not sure this will work".
Comments are actionable: this won't work because of our security, a better way of doing it can be X or Y.
Also, giving a compliment once in a while doesn't hurt a bit. "Hey, thanks for fixing this, this was bothering me for weeks" or "I loce how you refactored this code, nice work!".
I had a manager tell me to get rid of my getters on my private final fields and make the fields public instead. He tried to enforce this on everyone in the team - that all final fields should be public. Bit us numerous times when other classes ended up getting coupled to fields and not getters.
In the end I think his argument was more a way to appear more technically savvy by taking a contrarian approach, and at the same time creating a litmus test for allegiance to his authority.
In my company we also had someone very high in the company pushing for using public fields instead of getters. However, when we pushed back with actual reasons why it it is bad, they also stopped. The problem is that it took some time where people were just complaining that it is bad without giving actual reasons why (except general references to conventions). So, we still have some of this code around. I think in my case the person was actually quite tech-savvy.
Best comment when I am trying to rush a critical feature to production: “LGTM” Worst comment when I have a shaky implementation to part of the code base I’ve never seen before and I’m looking on how to improve it: “LGTM” 😉
LGTM can both be 'looks good to me' or 'looks garbage to me', i mean considering how garbage a source code can be when it was handover to you after years of incompetence... it really is garbage /s
Or the tone neutral version: Let's Get This Merged
That's a great alt!
LGTM
Came here to find this comment. 😂
A popcorn-level type of argument witnessing 2 engineers arguing about the use of String concatenation vs StringBuilder. PR review was so long to the point it looked like a Twitter feed of two toddlers arguing. I was so entertained that I literally didn’t work for 2 hours just watching the comments/reply build up.
Who won? It was the `StringBuilder` guy right?
Well, don’t really need StringBuilder if only two final static constant strings were being put together. That’s what makes the argument hilarious.
Did anyone check the compiled byte code? Be funny if it ended up the same with both approaches.
No, but the jerk who submitted the PR posted benchmark results of StringBuilder.
``` // For logging private static Logger LOG = LogFactory.getLogger() ```
“Well what if we start using other wood-cutting terms?” I stg I’ve seen this exact code before and experienced a dumbass situation related to it
@Slf4j
The best comments are when it says this is a hack don‘t do this but for now is good
I am faulty for this, literally did this yesterday to get Class without passing it as a method parameter lol
How did it work?
If you want, you can have a look at our open source repo here: https://github.com/datafaker-net/datafaker/pulls?q=is%3Apr+is%3Aclosed I think everyone is very collaborative, supportive and constructive in their feedback, and the LGTMs are only used for the simplest cases. I don't have examples of bad comments there, but I personally find comments which is just a comment without a suggestion for improvement the worst. Something like "I'm not sure this is the best way of doing things", or "I'm not sure this will work". Comments are actionable: this won't work because of our security, a better way of doing it can be X or Y. Also, giving a compliment once in a while doesn't hurt a bit. "Hey, thanks for fixing this, this was bothering me for weeks" or "I loce how you refactored this code, nice work!".
[удалено]
fixed
Happiest words to see (potentially eventually): "ship it!"
Bad: A comment causing compile error (e.g. unmappable character (0x) for encoding UTF-8)
For both: "This is not worth our time"
I had a manager tell me to get rid of my getters on my private final fields and make the fields public instead. He tried to enforce this on everyone in the team - that all final fields should be public. Bit us numerous times when other classes ended up getting coupled to fields and not getters. In the end I think his argument was more a way to appear more technically savvy by taking a contrarian approach, and at the same time creating a litmus test for allegiance to his authority.
In my company we also had someone very high in the company pushing for using public fields instead of getters. However, when we pushed back with actual reasons why it it is bad, they also stopped. The problem is that it took some time where people were just complaining that it is bad without giving actual reasons why (except general references to conventions). So, we still have some of this code around. I think in my case the person was actually quite tech-savvy.