> @mention Yes, GitHub would love for their namespace to be the implied namespace in commit messages. In other words, please don't. Use names or e-mail addresses to refer to people.
They're talking about pull requests, not commits.
How to write the perfect pull request (2015)
21–30 of 40 posts
Re: How to write the perfect pull request (2015)
#22> Ask, don’t tell. (“What do you think about trying…?” rather than “Don’t do…”)
When I led a team of 6 people, I did the opposite. The guideline was to always use the imperative form if you knew there was a better way, especially if there's a precedent or it's a written rule in a guideline, but even for opinionated things.
It resulted in clear debate in PRs, ZERO personal conflicts, everything was very civil and productive.
I'm just mentioning this as a personal anecdote, not suggesting that there's a right or wrong way. The reason this worked in my team is because we were, and still are - a team. They're still doing it like that even though I'm no longer involved in day to day.
Re: How to write the perfect pull request (2015)
#23> Ask, don’t tell. (“What do you think about trying…?” rather than “Don’t do…”) When I led a team of 6 people, I did the opposite. The guideline was to always use the imperative form if you knew there was a better way, especially if there's a precedent or it's a written rule in a guideline, but even for opinionated things. It resulted in clear debate in PRs, ZERO personal conflicts, everything was very civil and prod…
> The guideline was to always use the imperative form if you knew there was a better way, especially if there's a precedent or it's a written rule in a guideline, but even for opinionated things.
Same here. We used imperative form too and never had any problems. All developers were matured enough to not take comments about technical stuff personally. I think as long as we are being direct and polite in the reviews, there should be no issues.
Re: How to write the perfect pull request (2015)
#24> Ask, don’t tell. (“What do you think about trying…?” rather than “Don’t do…”) When I led a team of 6 people, I did the opposite. The guideline was to always use the imperative form if you knew there was a better way, especially if there's a precedent or it's a written rule in a guideline, but even for opinionated things. It resulted in clear debate in PRs, ZERO personal conflicts, everything was very civil and prod…
I agree.
It shouldn't disguise as a question if you're not really inviting a debate, doesn't feel honest IMO. When the imperative form is used the reader will often assume you know something they don't, and only argue when they see a problem or want to validate the assumption.
I believe the more the comments (annotations?) are about the code itself ("This should do X", "This is not doing Y") rather than a back-and-forth conversation ("What do you think about X?", "Have you thought about X?"), the less personal it feels, and the more the ego gets out of the way.
Same thing for denying PRs with "request changes" - I've had a few people say they avoid doing it because it feels aggressive, but knowing there's a problem and still letting it into production (and then have the PR author fix a much more costly mistake) isn't honest or professional.
Re: How to write the perfect pull request (2015)
#25> Ask, don’t tell. (“What do you think about trying…?” rather than “Don’t do…”) When I led a team of 6 people, I did the opposite. The guideline was to always use the imperative form if you knew there was a better way, especially if there's a precedent or it's a written rule in a guideline, but even for opinionated things. It resulted in clear debate in PRs, ZERO personal conflicts, everything was very civil and prod…
> The guideline was to always use the imperative form
Has this team been distributed in some way or did it consist of people that have regular in-person interaction?
Re: How to write the perfect pull request (2015)
#26> Use emoji to clarify tone. Compare “:sparkles: :sparkles: Looks good :+1: :sparkles: :sparkles:” to “Looks good.” Or, you know, write "Looks great!" Exclamation mark is the OG emoji
Or formalize phrases. The Google review guide [1] defines some phrases and as such there is no question about the tone of it. Prefix comments on non-essential parts of the commit with "Nit:", approve the changes with "LGTM" (looks good to me). [1] https://google.github.io/eng-practices/review/
That's boring. As a form of protest, I'm going to do all future reviews in emoji only.
Re: How to write the perfect pull request (2015)
#27> Ask, don’t tell. (“What do you think about trying…?” rather than “Don’t do…”) When I led a team of 6 people, I did the opposite. The guideline was to always use the imperative form if you knew there was a better way, especially if there's a precedent or it's a written rule in a guideline, but even for opinionated things. It resulted in clear debate in PRs, ZERO personal conflicts, everything was very civil and prod…
Granted this is from my experience of working in in-house dev teams (rather than say open source contributions), but very often I find there is no single objective right way to do something, certain things are just subjective opinions, and I'm not going to pretend that after looking at a PR on GitHub.com, that I know more than the person working on it (of course, there are exceptions, but this is what I find to be true generally).
If I see something that looks wrong, my assumption is that I'm missing some kind of knowledge or context that lead to the way for the code to be written the way it is. So I ask questions or offer suggestions or my thoughts. I'm definitely not going to tell someone to do something.
But at the end of the day, very rarely do I see something that should be a critical blocker for a piece of work, so I'll share my thoughts, approve, and leave it to the perfectly capable original submitter to do with that what they want.
Re: How to write the perfect pull request (2015)
#28Earlier quoted context omitted.
If you share a development branch with someone, then you should prefer merging over rebasing because rebasing changes rewrites the commit history, causing the locally checked out branches among collaborators to disagree. You never know when someone will need to take your branch to develop on, so generally it’s a good idea to merge over rebase. When master is merged into your PR branch, the fork point is forwarded to…
> If you share a development branch with someone, then you should prefer merging over rebasing Your parent comment is suggesting rebase only for pulling latest changes in master into your pull request. For merging someone's pull request to the team's master, sure use a merge commit. But if you are working on a pull request and while you are working on it, the team's master gets updated and now you want to base your w…
Using a rebase to update from master will cause conflicts for anyone else working on the PR branch. To understand why, imagine a developer branches from master at commit A, and then creates two commits on the branch B and C. In the meantime, master gets updated with commits D and E. Rebasing in this situation, results in a branch that looks like A, D, E, F, G, where F and G contain the same content as B and C, but are now tracked under different hashes. If a collaborator has this branch, git will report the local branch has two different commits (B and C) and that the remote branch has two different commits (F and G). Doing a typical “git pull” in this situation will lead to merge conflicts in everything touched by commits B and C. You can work around this specific problem by using git pull —rebase. However, git works best when the remote history is immutable. If you have a specific commit in your PR that you want someone to look at, you send them a link to it using the commit hash. Once you rebase, that commit hash will no longer point to anything. Rebasing is useful for completely changing the branch your PR branch is cut from. But once you rebase from one base branch to another, you should then continue to merge from the new base branch.
Re: How to write the perfect pull request (2015)
#29> Ask, don’t tell. (“What do you think about trying…?” rather than “Don’t do…”) When I led a team of 6 people, I did the opposite. The guideline was to always use the imperative form if you knew there was a better way, especially if there's a precedent or it's a written rule in a guideline, but even for opinionated things. It resulted in clear debate in PRs, ZERO personal conflicts, everything was very civil and prod…
This can be culturally biased as well. I find European groups tend to be more assertive where as east Asian groups definitely don't use imperative forms as a matter of course. I think the takeaway is to always consider the audience for which you're writing.
Re: How to write the perfect pull request (2015)
#30> Ask, don’t tell. (“What do you think about trying…?” rather than “Don’t do…”) When I led a team of 6 people, I did the opposite. The guideline was to always use the imperative form if you knew there was a better way, especially if there's a precedent or it's a written rule in a guideline, but even for opinionated things. It resulted in clear debate in PRs, ZERO personal conflicts, everything was very civil and prod…
> The guideline was to always use the imperative form Has this team been distributed in some way or did it consist of people that have regular in-person interaction?
The entire team is within a 2 meter radius :)
There are other teams within the department which are distributed in a total of 4 cities inside the same country (a sort of "hub and spoke") and with SOME of these teams, because they collaborate and communicate often, this candid communication works well.