| |
| ▲ | fmbb 20 hours ago | parent [-] | | What does this mean? A Merge Request or Pull Request is the only thing that makes sense to review, and always has been since before LLMs. If some author tells me “you can review each commit to make it easy” I will ask them to make separate PRs. | | |
| ▲ | soon_to_be_sb 17 hours ago | parent | next [-] | | >A Merge Request or Pull Request is the only thing that makes sense to review, and always has been since before LLMs. Why doesn't it make sense to review smaller units of change within a big PR? | | |
| ▲ | skydhash 17 hours ago | parent [-] | | Not GP, but in my experience, those small changes are usually contrived to be small. Like saving snapshots instead of being atomic and self contained. And if they are the latter, they should have been separate PR. That’s why I usually look at the diff of the whole PR and prefer to merge by squashing. Commits inside the PR are usually noise. | | |
| ▲ | soon_to_be_sb 15 hours ago | parent [-] | | What you are saying is true if created manually. It is a chore to create sensible commits messages when you are committing often. But LLMs appear to be pretty good at making small commits with sensible commit messages, that greatly help to make sense of the whole PR. Regarding large monolithic commits, how do you even know where to start reviewing in a large PR? Smaller commits at least give you some sense of ordering. | | |
| ▲ | skydhash 12 hours ago | parent [-] | | > Regarding large monolithic commits, how do you even know where to start reviewing in a large PR? What I’m saying is that large commits shouldn’t be a thing without out of band discussions (formatting, design refactoring, features merging). With such knowledge, it’s easy to glance at the code and understand the implementation. | | |
| ▲ | soon_to_be_sb 3 hours ago | parent [-] | | >without out of band discussions.. Not every project can afford this. >With such knowledge, it’s easy to glance at the code and understand the implementation. I really doubt that this is true for large, legacy code bases, where one is only familiar with a small part of the system. | | |
| ▲ | skydhash 2 hours ago | parent [-] | | > Not every project can afford this. You expect to send a big PR to someone without him being aware of the context or without being available to discuss your solution? > I really doubt that this is true for large, legacy code bases, where one is only familiar with a small part of the system. Reviewers should already be knowledgeable about the part being modified. If no, the review is just for checking for antipatterns in the code, or badly written tests. A bit useful, but can be unnecessary in high trust (or keen on accountability) workplaces. |
|
|
|
|
| |
| ▲ | watwut 18 hours ago | parent | prev [-] | | It means you gave up on getting claude to show you changes as it is doing them, in a comfortable form, so that you can steer it as it is doing them. |
|
|
