Like the other guy, I also read your comment twice looking mistakes but found none.
You should of left something to fix!
😏
Comment on When someone corrects your code
aeki@slrpnk.net 8 months ago
I am really happy when people are quite strict in code reviews, it makes me feel safer and I get to learn more.
Nothing worse than some silent approvals with no real feedback. What if I missed something obvious… and now it’s merged.
To be fair, I also enjoy getting my grammar corrected. I’m juggling 3 languages and things can get messy.
reflex@kbin.social 8 months ago
ironhydroxide@partizle.com 8 months ago
Correcting the reviewer.
Notes: “should of” isn’t valid, should implies a verb, of isn’t a verb. I expect you meant “should have”. Please recall this in future submissions.wandermind@sopuli.xyz 8 months ago
They should of course keep that in mind, but it’s not that “should” should always be followed by a verb directly. The problem is that “of” in this context is a mishearing/spelling of “have”, so they should in this case have written it like that instead.
Huschke@programming.dev 8 months ago
I love that you used “should of” in a valid sentence.
jadero@programming.dev 8 months ago
I would argue that “should of” is just a naive written rendition of the spoken contraction “should’ve”. They are homophones, so it’s a completely understandable error among those without the relevant education or background. I know only English and was in Grade 9 at a different school before someone corrected me.
reflex@kbin.social 8 months ago
Notes: “should of” isn’t valid, should implies a verb, of isn’t a verb. I expect you meant “should have”. Please recall this in future submissions.
😏
interolivary@beehaw.org 8 months ago
should implies a verb, of isn’t a verb
“should” and “of” should probably be in quotes here?
LinuxSBC@lemm.ee 8 months ago
A question mark does not fit the sentence. While it is commonly used to demonstrate a rising tone at the end of a sentence, its not considered correct for formal writing.
interolivary@beehaw.org 8 months ago
Notably, a good code review should also bring up the good parts of the submission, and not just concentrate on the errors. Not only does it make the recipient feel better to get positive feedback among the negative, but it helps them learn about good practices too.
Many reviewers concentrate on just finding mistakes, and while it’s useful it’s sort of the bare minimum; a good code review should be educational. Especially if the submitter’s a more junior coder, in which case it’d also be a good idea to not just outright tell them how you’d fix some problem, but sort of lead them to a solution by asking them questions and pointing things out and letting them do the thinking themselves. But still, even experienced coders benefit from good feedback
aeki@slrpnk.net 8 months ago
Yes, I tend to do that, and thankfully some of my colleagues do too. Clever but readable solutions, following good and relevant practices, clear documentation, making a good MR description that makes it easier to review, and more.
interolivary@beehaw.org 8 months ago
That’s great to hear. It’s thankfully becoming more common in general, and we can all do our part in spreading these practices.
I tended to actively evangelize for it when I was managing coders or teams. Unfortunately it’s still not all that uncommon for coders to be downright offensive when giving feedback, like not necessrily quite Linus-level rants but things like “this is idiotic, this is stupid, that’s shit, why would you do that” etc etc. The usual explanation I’ve gotten is that they’re just being “honest” and saying what they think, and it’s not their problem if the reviewee (is that even a word‽ I can’t English today) gets offended. Some even get all huffy about it, like “oh we’re just supposed to coddle them and never say anything negative so their little feefees don’t get hurt?” And I mean, yeah, getting honest feedback definitely a good way to learn, but it’s not like the only way to point out errors or problems is to be a cunt about it.
Doug@midwest.social 8 months ago
I’m juggling 3 languages
We Americans like to forget that anyone might have any trouble understanding English especially in cases of polyglots.
I don’t know which is your native tongue but from this comment it looks like you’re doing a fine job.
CoderKat@lemm.ee 8 months ago
Yeah, I learn so much from code reviews and they’ve saved me so much time from dumb mistakes I missed. I’ve also caught no shortage of bugs in other people’s code that saved us all a stressful headache. It’s just vastly easier to fix a bug before it merges than once it breaks a bunch of people.
jadero@programming.dev 8 months ago
In that spirit, I will call attention to your first sentence, specifically the comma. In my opinion, that can be improved. One of three other constructions would be more appropriate:
The first of my suggested changes is favoured by those who follow the school of thought that argues that written sentences should be kept short and uncomplicated to make processing easier for those less fluent. To me, it sounds choppy or that you’ve omitted someone asking “Why?” after the first sentence.
Personally, I prefer the middle one, because it is the full expression of a complete state of mind. You have a feeling and a reason for that feeling. There is a sense in which they are inseparable, so not splitting them up seems like a good idea. The “because” explicitly links the feeling and reason.
The semicolon construction was favoured by my grade school teachers in the 1960s, but, as with the first suggestion, it just feels choppy. I tend to overuse semicolons, so I try to go back and either replace them with periods or restructure the sentences to eliminate them. In this particular case, I think the semicolon is preferable to both comma and period, but still inferior to the “because” construction.
I’ve clearly spent too much time hashing stuff out in writers’ groups. :)
aeki@slrpnk.net 8 months ago
This is what I live for. :D
I agree with most of that. In formal settings, I prefer full sentences with conjunctions; however, choppy sentences are the ones that often end up in my Lemmy comments.
jadero@programming.dev 8 months ago
That only makes sense. We are having a conversation, not creating literature.
little_hermit@lemmus.org 8 months ago
Strange, I get a mild hostility vibe from colleagues if I review too ambitiously.
jadero@programming.dev 8 months ago
Reviews have to be balanced to circumstance. There is a big difference between putting out the sales brochure and the notice on the bulletin board. Likewise in coding a cryptographic framework for general consumption and that little script to create personal slideshows based on how you’ve tagged your photos.
As a general rule, wider distributions, public distributions, and long-lived distributions need more ambitious reviews. If the distribution is wide, public, and permanent, then everything needs very detailed scrutiny.
I have found some success in starting with and occasionally revisiting review goals. This helps create and maintain some consistency in a process that is scaled to the task at hand.