Hey, so I feel we are suffering from a review bandwidth deficit. I want to help with this, by documenting more of the "internal invariants" and "how to do review" stuff.
I am putting info here: https://github.com/rust-analyzer/rust-analyzer/pull/4703
Question: how do folks feel about reviewers pushing commits directly to the PR branch (allow edits from maintainers)?
I've noticed that the way I do thorough review is by checking out the code locally, and testing if my suggestions compile. So, in a sense, I first apply suggestions locally, and then write it again, as the comment on the PR.
I wonder if it makes sense to just directly push the suggestsions (preserving the same "motivation" part I would to GH comment to explain why this change is a good idea in my options)
I think it should be allowed but would also say that I've learned a lot of rust from you telling me my PRs are wrong and having to type it all out myself :)
Yeah, I guess, that's the main question -- would pedagogical value of reading someone else's commit be comparable to typing the stuff yourself...
When you type something yourself, you are forced to consciously think about it. When you read a commit you might just skim it without paying too much attention to the actual changes.
I'd go for code snippet suggestions in PR itself. Pushing directly to PR branch you don't leave space for objections
Code snippets are nice but I find that I'm tempted to make suggestions without testing them
which I guess could be good pedagogically :laughing: but also means I could suggest something that fundamentally doesn't work