183
votes
Accepted
Should I submit a pull request to correct minor typos in a Readme file?
Just fix all the typos you noticed and create a pull request with a comment along the lines of 'Fix typos'. Then it's one button to click for a person with the correct access.
You don't need to ...
- 1,710
56
votes
Should I submit a pull request to correct minor typos in a Readme file?
Some context that may or may not be relevant.
A cloud hosting provider named DigitalOcean hosts an event every year called Hacktoberfest to encourage people to contribute to open source projects in ...
- 1,628
31
votes
Accepted
I accidentally overhauled someone's entire project. Any acceptable way to pull request?
If the project was "5 years untouched" as you wrote, it is likely that pull requests are not going to be accepted, regardless if someone fixed a typo in a comment or did a complete rewrite. The ...
- 220k
27
votes
Accepted
How to handle a TODO in a pull request?
When you say that they "generally stay in the codebase for the lifetime of the codebase" in your team/department/organization, consider the following:
Write it down in your DoD that TODO, FIXME, or ...
- 473
15
votes
Should I submit a pull request to correct minor typos in a Readme file?
Correct the typos and submit a pull request with the position of each correction in the summary field.
That information is already visible when looking at the changes themselves, and any pull request ...
- 59.5k
11
votes
Accepted
If PRs are not being approved, when do you stop branching off of master?
I have a lot of code that is not merged to master/main
This is a "management smell". It signifies that continuous integration has not been so continuous. Sometimes it's unavoidable - there'...
- 15.3k
10
votes
How to manage pull request review and approvals
Reviewing pull requests is part of your team's work. If you as a team decided that PRs are needed (and that they need to be reviewed by 2 developers), then they are just as important as writing the ...
- 5,586
8
votes
Accepted
Is it necessary to understand the requirements of a change in order to perform an effective code review?
One of the main reasons for code review is "does it do what it is supposed to do?" Without knowing requirements (and, thus, intent) for the code you can only do a formal analysis of the code ...
- 958
8
votes
Accepted
Should PR reviews check for code correctness?
There is more than one way to shave a cat.
In an ideal organization which has a good testing process, and where only experienced developers work who regularly write unit tests, you can leave ...
- 220k
7
votes
Code review workflow where pull request author must merge
Having the initial author merge their own pull request is my preferred workflow in small teams. In addition to technical advantages already mentioned (in terms of resolving merge conflicts, for ...
- 171
7
votes
Should I submit a pull request to correct minor typos in a Readme file?
Not that this specifically answers the question, as there are already answers, but here is the general advise I give people on commits and messages.
Break up commits into logical chunks. The example I ...
- 179
7
votes
Are small pull requests better than one large all-encompasing features, and how can I convince my team of that?
Note that without concrete numbers and sizes, it's hard to give you an objective judgment on who is wrong and who is right. Maybe their tickets are too big, maybe you're making your changesets too ...
- 59.5k
7
votes
How can we reduce the PR load on our team?
In this case each PR has to be approved by five reviewers.
This is your biggest problem. Reduce this number to two or at absolute maximum, three. If you don't trust that two developers on your team ...
- 26.1k
5
votes
Accepted
How do pull requests fit into a development process?
A pull request is the same as having a code review when code is "merge ready". Whenever you have a code review and merge, replace that with a pull request and you have successfully fit a ...
- 85.9k
5
votes
Accepted
What is the most effective way to explain code in a code review using a pull request?
There are a few options.
One option is to utilize GitHub issues and ensure that every PR is associated with an issue. This gives you the opportunity to prioritize work, a singular place to talk about ...
- 85.9k
5
votes
Why squash git commits for pull requests?
I would see if the code is going public or not.
When projects stay private:
I would recommend to not squash and see the making of the sausage. If you use good and small commits, tools like git ...
- 163
5
votes
How to handle a TODO in a pull request?
TODO's them selves are not evil. I am a big fan of never going more then one layer deep, and always fixing 1 issue per tracker ticket.
I often use TODO or FIXME as a way to remind my self that I ...
- 2,583
5
votes
two level code review using git pull request in BitBucket
I see two ways you could handle this.
Set up a second pull request after the first one is successful, making sure not to complete the first PR and merge it in. Downside here is the possibility that ...
- 5,586
5
votes
Waiting on Multiple GitHub PR Merges
Create your 'continue working' branch (whatever that's going to be called), and cherry-pick the other PR changes in. (You could merge, but it'll get messy if you later need to address review comments ...
- 249
5
votes
Accepted
Is there any advantage of pushing branches to GitHub without an associated PR?
Small features / bugfixes can be added, tested, pushed, and a PR created all within a short time span - maybe a couple of hours. In those cases, there's not much reason to push a branch without ...
- 5,586
4
votes
Working on issue that relies on changing code
There's no solution* for the underlying problem of "what if someone changes the code I was working on before I finish my changes", you can only manage the process.
Traditionally feature branches ...
- 84.4k
4
votes
two level code review using git pull request in BitBucket
Bitbucket has a policy feature which can enforce two-levels of review without any process changes:
The easiest policy is to enforce that a few people look at the new feature or bug fix before it's ...
4
votes
Is it necessary to understand the requirements of a change in order to perform an effective code review?
It’s limited. Now the code will go through QA so if it doesn’t do what it is supposed to do it will be stopped, and I could assume that you make mistakes, but don’t do something outright stupid. A ...
- 49.4k
4
votes
Is it necessary to understand the requirements of a change in order to perform an effective code review?
Somewhat. General code style and approach is something that you don't need to know context for.
However, it's perfectly possible to write style-compliant code that doesn't achieve the goal; and ...
- 59.5k
4
votes
Should PR reviews check for code correctness?
If you attempt to double check code in PRs you are effectively writing the code yourself and will end up having no time for anything but PRs
Instead decide what the rules for passing a PR are and ...
- 84.4k
4
votes
Aiding code review with an additional pull request
The question is: Is the pull request, as it is, acceptable? Not perfect, and not as good as you want, but acceptable?
In that case, if everyone agrees, you could accept the pull request, merge it ...
- 49.4k
3
votes
Pull requests as knowledge transfer: an anti-pattern?
To me, PRs are not a suitable vehicle for teaching domain knowledge. PRs are too random to do that (there is a poor signal to noise level when it comes to interesting information nuggets), the ...
- 2,947
3
votes
Accepted
two level code review using git pull request in BitBucket
I do this quite frequently, for microservices other teams own, or for documentation changes to tech pubs. We have separate repos for individual microservices, with only a scrum team or two having ...
- 149k
3
votes
Why squash git commits for pull requests?
Because of the perspective ... The best practice is to have a single commit per single <<issue-management-system>> issue if possible.
You could have as many commits as you want in your ...
- 169
3
votes
Working on issue that relies on changing code
If you always have a time lag of 24 hours before you get feedback on a pull request, and you run into trouble if it is rejected because your next work depends on it, you want to make it very likely to ...
- 220k
Only top scored, non community-wiki answers of a minimum length are eligible