lf you have more than 10 rules / conventions in your code base and people can't get them right by hand it's a smell your codebase is entertaining OCD and will never have good reviews that catch bugs or feature spec mismatches. Linters just legitimize stupid shit like single quote vs double quote, never align assignments, always have a space after a round bracket but not a square one and a litany of other I-Never-Learned-What-Matters stuff.
@HYRULE10
4 күн бұрын
My team typically prefixes small comments like that (assuming they're not caught by a linter) with "nit: thing could be more descriptive" so that the reviewee knows that these are minor things. Then when there's a comment that doesn't have a nit, they know that's more important feedback to consider.
@nuttygold5952
3 күн бұрын
Same
@TheStruders
Күн бұрын
If a comment can be ignored or is optional, then it's a waste of time adding it. The whole point of the video is these "nit picks" should and can be automated away
@HYRULE10
Күн бұрын
@@TheStruders a linter can't tell you that your variable isn't describing the right thing. saying green = yellow isn't something a linter is going to find.
@nuttygold5952
Күн бұрын
@@TheStruders a comment might be optional if, time does not allow or the implementor has a good reason not to do it. For example, lets say were moving in this new direction and any new code should be migrated, you as a senior might want to share that knowledge and inform the implementor but not enforce because maybe the refactor is just too big to do at that point. You tone implies you have a hard and fast rule, which is almost certainly wrong some of the times!
@TheStruders
Күн бұрын
@HYRULE10 if a variable is describing the wrong thing and requires changing, then that is not a "nit pick", that is a valid PR comment that requires a change
@potaetoupotautoe7939
8 күн бұрын
i have absolutely seen the same problem at my previous job. love your content. please upload more knowledge.
@dstn3422
11 күн бұрын
Well, most developers will think about PR reviews as a chore. Everyone wants to code but not everyone wants to review other's code. So this might be a mentality problem too. I've left 12 (it was 12 btw :D) comments, I did my part. But also, I wouldn't say that the review given sucks in such case. Yes, you lost focus as a reviewer of what's important because you've been distracted by smaller issues, but I'd blame it on the person creating the PR. Even as a junior, are you really not able to follow naming conventions that are already established in the code base? I don't want to introduce a thousand linter rules for each case. I want people be able to follow established project styles.
@axisaligned9799
10 күн бұрын
if your *many* tiny rules can't be introduced by a linter & formatter then you are just creating arbitrary rules that follow no pattern...
@dstn3422
8 күн бұрын
@@axisaligned9799 Yes, definitely, we can go to either extreme. Having too many arbitrary rules is the other end of the spectrum.
@Egzvorg
7 күн бұрын
Raymond Hettinger gave a talk "beyond PEP8" for Python. There is a great experiment he shows to demonstrate the distraction problem (you need to count basketballs on screen).
@sp3ctum
12 сағат бұрын
Linters are great! There's even react specific ones available for eslint. I think in this example react-hooks/exhaustive-deps would have complained because the useEffect hook seems to be missing a dependency to the user id. If the user id changes, the useEffect doesn't get re-run and can show stale data. Linters like these are a huge time save!
@stefanalecu9532
4 сағат бұрын
Especially if you combine them with glorified linters such as TypeScript
@piotrlewandowski
2 күн бұрын
Pity you didn't address the most import thing here: fetching data in useEffect
@someaccount-mp4tk
Күн бұрын
This guy right here == gold advice
@hodayfa000h
16 сағат бұрын
True or false?
@trigunadi4024
7 күн бұрын
do people want to spend time to review code?
@arnthorsnaer
5 күн бұрын
Yes, code reviews are great for devs of all skill levels to learn new things, teach others but last but not least to help devs be aware of the trends in the codebase.
@svrls0619
3 күн бұрын
Don't get me wrong sir, I have just idea, that the way you talk about those CR rules relates to usage of JS and VSCode. I apologize, I did not mean it wrong or bad, just idea to share. PS. I actually like how reflecting (on the other hand) you are towards your code.
@davidhart1578
3 күн бұрын
Yeah that's fair, JS is so I unopinionated that there are a lot more syntax "mistakes" it allows. A more strict language simply wouldn't compile this kind of thing. The video itself was inspired by a PR I reviewed that had lots of "nitpick" comments but no one spotted a major issue in business logic. It's a bit contrived as I can't show actual code from my employer's app. Thanks for the thoughtful feedback!
@SonAyoD
7 күн бұрын
Great video
@devoiddude
7 күн бұрын
I prefer javascript, never saw the value in typescript personally.
@SonAyoD
7 күн бұрын
Wow
@devoiddude
6 күн бұрын
@@SonAyoD I see we have a typescript fan boi here.
@StellaEFZ
6 күн бұрын
Why don't you feel the value in type safety?
@SonAyoD
6 күн бұрын
@@devoiddude im just interested in your reasoning. I could never build any medium to large system without type safety.
Пікірлер: 32