Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

The impact of file position on code review

arxiv.org

Post reply on HN
Print view View original

21–30 of 38 posts

Re: The impact of file position on code review

#21

Post by zoover2020 »

jsbg wrote: post #14

> We found files shown earlier in a PR to receive more comments than files shown later I often stop reviewing a PR if I have too many comments on it already that need to be addressed so I wouldn't say this is necessarily what it sounds like (later files get less attention). Also, a comment on an early file might address a concern in a later file that will be fixed by the time I actually review the later files. That s…

Can't seem to get anyone else to Baird the train of small CRs. It's so obvious

Re: The impact of file position on code review

#22

Post by FajitaNachos »

progbits wrote: post #17

People review files top down? I can't imagine doing that. I glance at the list, and try to spot the highest level change (schema changes, interfaces, etc) and start there. If the core idea is wrong no point wasting time on the rest. Then I go up from there to implementation details and tests. Often jumping between modules or functions, coming back to some things later when I have more context.

I would bet the vast majority of people do this. If they didn't, or preferred something else, it wouldn't be presented in the UI this way.

Re: The impact of file position on code review

#23

Post by teddyh »

kridsdale3 wrote: post #13
aerostable_slug wrote: post #7

Note to self: bury malicious code in "later"-reviewed files.

Similar, if you're going up against a promotion committee, or parole board in jail, you want to go when the judges have been recently fed.

Wasn’t that debunked?

Re: The impact of file position on code review

#24

Post by stillpointlab »

kevinsync wrote: post #15

IMO file/folder structure in a project is a geographic mapping exercise. It's not that far off from city planning. I think it's so important to design the structure and naming conventions of a project with the ultimate goal of aesthetically-pleasing, human-understandable spatial organization of digital artifacts. You "go here" to "see that". This "lives here". You travel to the destination of the thing you're looking…

I totally agree, and I use LLMs for this. I have a script that will output my directory structure (just the folders and filenames + the lines of code; aggregate for folders and per file). Then I ask the LLM to give me feedback. Surprisingly this has lead to some very useful feedback and improvements.

Re: The impact of file position on code review

#25

Post by gruez »

teddyh wrote: post #23
kridsdale3 wrote: post #13

Earlier quoted context omitted.

Similar, if you're going up against a promotion committee, or parole board in jail, you want to go when the judges have been recently fed.

Wasn’t that debunked?

yes https://nautil.us/impossibly-hungry-judges-236688/

Re: The impact of file position on code review

#27

Post by devnullbrain »

jsbg wrote: post #14

> We found files shown earlier in a PR to receive more comments than files shown later I often stop reviewing a PR if I have too many comments on it already that need to be addressed so I wouldn't say this is necessarily what it sounds like (later files get less attention). Also, a comment on an early file might address a concern in a later file that will be fixed by the time I actually review the later files. That s…

Read this as though it has a visibly bulging temple vein:

Does that mean you do the thing where I respond to your review, request a re-review, then get comments about code that was already there a week ago?!

Re: The impact of file position on code review

#28

Post by devnullbrain »

progbits wrote: post #17

People review files top down? I can't imagine doing that. I glance at the list, and try to spot the highest level change (schema changes, interfaces, etc) and start there. If the core idea is wrong no point wasting time on the rest. Then I go up from there to implementation details and tests. Often jumping between modules or functions, coming back to some things later when I have more context.

I think people putting enough thought and effort into reviewing, and developing it as a skill, are a small minority.

Re: The impact of file position on code review

#29

Post by devnullbrain »

slongfield wrote: post #9

I've recently started using Github code reviews for a lot of C++, and one thing that I wish it would do is show the header (.h) files before the implementation (.cc) files. Small PRs help, but I often end up just opening a handful of windows to have everything open at once.

For this reason I usually either check it out locally or use the Github IDE (. or >)

Re: The impact of file position on code review

#30

Post by devnullbrain »

rwmj wrote: post #10
slongfield wrote: post #9

I've recently started using Github code reviews for a lot of C++, and one thing that I wish it would do is show the header (.h) files before the implementation (.cc) files. Small PRs help, but I often end up just opening a handful of windows to have everything open at once.

You can't do that in github as far as I know, but you can get git to display diffs that way (eg. for local review or email based workflows). You have to use a git "orderfile". Example: https://gitlab.com/nbdkit/nbdkit/-/blob/master/scripts/git.o... We found it useful to display header files, interface files and documentation first, but maybe the linked paper will make us review that!

Github similarly lacks the ability to override the diff algorithm. This is a shame, because `histogram` makes some diffs look much more like what the human intended.

The default, Myers, is from 1986!

Post reply on HN