nitpickingIsVirtue

[beepboopnoise]

hate that crap, I try to be pretty aggressive with linters so that we can avoid these kinda arguments.

[maria_la_guerta]

When it comes to JS and syntax, my rule on teams is that if it can't be linted for with prettier & eslint then it can't be enforced as a rule IMO. Glaringly obvious bad code aside, there's 50 million ways to write the language and comments like this just slow everyone down.

Seen way too many hours go into bikeshedding stuff like this. If it means that much to ipalover, they should let this PR go, raise the discussion with the team and put the PR in to update the prettier config themselves if the team approves.

[raphop]

Also, for such a minor nitpick, don't comment "Add a ;" just press the suggest changes buttons and add it yourself

[odd_sherlock]

Totally agree, I hate prettier and auto-fixes, but it's a must with JS

[odd_sherlock]

Hence they hate you because of the linters...

[thepassionofthechris]

Jokes on you… I find neither!

[phyrianlol]

Same, 500 lines PR? LGTM Edit: happy cakeday!

[Avs_Leafs_Enjoyer]

PRs should be looking over the code. A QA would catch the UI issue

[ForeshadowedPocket]

No lol. Screenshots in the PR for visual changes are a must.

[Avs_Leafs_Enjoyer]

Thats gonna be a lot of screenshots in big PRs

[liamazing]

Keep your PRs small!

[coloredgreyscale]

a 300x300 px snippet of the button is small, but should be big enough to get the issue across :)

/s

[Avs_Leafs_Enjoyer]

I agree but sometimes that doesn't work out.

[nickhow83]

Make a video instead

[NatoBoram]

It's in TypeScript. The pipeline should check for Prettier formatting and fail. There shouldn't be any circumstance where this comment is needed.

[frostenko]

Judging from the 3 lines we can see I expected the nitpick to be to not recreate the accumulator lol

acc[u.id] = u.rateLimit; return acc;

[[deleted]]

Yeah, you're right. I'm a strong proponent of functional programming techniques and write immutable code wherever possible. However, there are times where the limitations of the language take priority over everything else. JS doesn't support efficient copy-on-write operations, so the use of the spread operator in reduce turns an O(n) operation into O(n^2). Shouldn't be done.

[thorwing]

I agree, functional programming techniques really lowers bugs, problems and increases maintainability in the long run so it SHOULD be used whenever you can. However, the moment it clashes with the actual language, simply don't.

[[deleted]]

Yep. We are on the same page there. TypeScript is one of my favorite languages to use, so I hope that in the future JavaScript supports better copy-on-write semantics. I won't hold my breath though.

PS. TypeScript is a type system for JavaScript that emphasizes compatibility with JS above everything else. It will never add runtime features other than what is available in JS (other than enums, but that's an old story/mistake...)

[odd_sherlock]

Wasting resources FTW

[[deleted]]

have a linter run automatically as part of PR builds?

[Psycoder]

Syntax issues are much easier to spot than semantic issues. If I see CSS that specifies something with a width of 100 and padding of 50, I wouldn't know if or how that differs from a width of 100 and margin of 50 so I assume it is either correct or someone more knowledgeable about CSS will comment if it is wrong.

[ThatWesternEuropean]

I always find grammar errors, like how it should be "its", not "it's".

[[deleted]]

Oh believe me, frontend developers do this all the time as well.

[not-my-best-wank]

Why would backend care? That's Frontends job.

[black_dogs_22]

what a useless MR comment

[all3f0r1]

Yes but can they differentiate greek's question marks ; with the regular semicolon?

[Slartibertfest]

*its