Ask HN: How to efficiently review a pull request that is full of linter changes? | Hacker News Reader

Ask HN: How to efficiently review a pull request that is full of linter changes?

1 pointzikohh12 comments

A lot of the changes are just lines have been reordered and within context, this isn't useful to me.

Any tips to review a large pull request consisting of 1k lines changed all due to a new linter being applied.

It's a public repo or a PR inside the company where you work?

Anyway, the solution is to ask nicely that the author rewrites the PR with the relevant changes, and then make another PR with the new linter. (Which linter to use, is a project-wide decision. Who is in charge of this?)

The PR only contains linter changes. However, there are so many it's very difficult review. I've nearly finished but I hate doing things manually and therefore was trying to find a more efficient way to do this in future

It's an internal repo. Not public

If there's a recipe to produce the change, ask for the recipe instead (and better yet, put it in the commit.) Then if you can reproduce that PR, and tests are passing, skim through it and see if nothing funny. Then it should be good to go.

I'd like to add the most linters change things like line length and so on. However, this linter is changing the order of keys and values in a go struct (which I'd already formatted with go fmt), which makes it much harder to review than your typical line length change.

> this linter is changing the order of keys and values in a go struct

Is this a good idea?!?! I don't know golang. Can you show an example where this change is useful? (Or a link to a webpage that explains this change.) [Sorry for the side question, but I'm curious.]

https://pastebin.com/Cdif0t5v

It changes the initialisation order - it's not a problem but hard to review.

Oh wait useful ?! I don't think ordering structs alphabetically is useful I agree that if it's not useful it shouldn't be done and transitively that makes the pull request easier to review.

But still a very indirect approach

Can it be disabled with an option? If I have an structure with mass, height and width, I'll really hate to order them as height, mass and width.

My totally uninformed opinion is to run away from a linter that makes that changes.

tldr: don't review linter changes

Run the new linter on all the files. Don't make any other changes. Spot check a few places to make sure the linter doesn't majorly mess anything up. Make sure your automated tests still pass. Merge this change without reviewing every single line.

Now that all your files are "clean", continue making changes and reviewing them like normal.

This has been my approach in the past but after reviewing it, I've found it to be sometimes inconsistent.

Consistently inconsistent. But it took me ages to figure this out. Hence me looking for an enhanced way to review

Reply on news.ycombinator.com