Square 公司怎么写提交信息
解释变化
在 Square 提交信息是很重要的交流形式,所以写好提交信息是很好的为大家节约时间的方式。我们常常希望以前能够做的更好。那些时候的提交信息仍然存在,并且很多仍然非常重要。
我们搜集了我们投入在出色的提交信息上的各种想法,这不仅是为了短期内节约我们的时间,从长远来看也可以。现在我们将它们分享出来希望同时也对你的团队有所帮助。
TL;DR: 优秀的提交信息解释了变化
在 Square, 我们编写了大量的代码。最好的代码应当是不言自明的。当我们的代码表述性不够的时候,就会有注释来解释这段代码做什么事情,或者更进一步,为什么它要这么做。一个开发者如果去阅读 SHA 的代码,他就能理解它做了什么事情以及为什么它要以那样的方式去完成。
提交信息不是关于代码的,它们是关于变化本身的。这是一个至关重要的差别,但是这个差别常常被忽视。
当我们进行提交时,我们会将一些理想状态下很清晰、自解释的代码覆盖其他一些很清晰、自解释的代码。不管修改前后的代码有多么清晰,这两者之间仍然会有内容的差别。提交信息就是建立在这个差别之间的桥梁。
提交信息有两个读者
考虑下提交信息的读者。一般分为两类:代码检查者和挖坟者。
直接读者应该是代码检查者,他需要决定是否接受这次改变,以及代码的改变是否反映作者的意图。代码检查者为了评估,则需要了解变化的背景,对他们而言,如果有信息可以为该代码变化提供背景知识,而不需要通过修改前后的代码来推断,会容易很多。
从长期看,提交信息的读者应该是某一位开发者,他尝试修复软件中的某一个问题、移除不再使用的代码、在现有的代码基础上开发新的功能、或仅仅是做出相似的改变、或是这些事情中的几个。有时候,某一位开发者尝试理解过去的一个决定(通常是作者本人!)。这就是考古学,如果每一条提交信息都有一条帮助性的信息解释这些改变,事情就变得非常容易。否则,挖坟者除了通过比较修改前后的内容来重建提交者的思考过程别无他法--这通常是不可能完成的任务。
事实上,它们有三个读者
出色的提交信息还帮助我们晋升我们最好的工程师。它们提供了一些内容和细节,可以帮助我们评估一个工程师日常工作的技术敏感度。同样重要的是,出色的提交信息展示了我们作为沟通者的技巧和同理心。清晰书写、有见解、有帮助的提交信息说明了一位工程师有远见并且能为他人考虑。有技术能力、同理心和有效沟通都是在 Square 获得晋升的关键因素。
怎么做
我们很明确
我们是在修复一个问题?我们会简单的描述预期和实际的行为,以及改变是怎么和问题关联的。我们是在添加一个新功能?我们会总结一下这个功能。
要假设读者对我们为什么要做出修改是一无所知的,这很有帮助。对多数读者来说,这是真实的---即使是最小的,最“明显”的改变。写下修改动机尤为关键。
我们很精准
这似乎是不言而喻的,但是在回应 review 或者其他的迭代过程中,很容易忽视提交信息的更新。为了防止不准确,提交者和检查者通常会在合并之前双重检查提交信息。
我们会说明为什么这次修改是安全的
尤其是我们在修改一些基础功能代码或者处理一些不同寻常的事情时,记录下为什么那么做是正确的,这是非常值得的。将我们的考虑和想法写到记录中,会给读者一些保障,以防万一以后再出现问题。
用更多的背景指向来源
Jira是肯定的。设计文档,google groups 中的跟帖,Slack 中的讨论也是合适的。和变化相关的背景我们提供的越多,挖坟者就越容易理解。
当然,简单的链接到一个 Jira 标签或者是讨论跟帖并不是非常有用。为什么要让读者(他们可能不是在浏览器中阅读这些提交信息)去其他地方了解这些变化呢?他们可能不耐烦,所以可能会丢失一些关键的细节。他们同时也可能无法查看这些信息!例如:曾经我们使用过 Github 企业版。现在 Github 所有的那些PR 信息都已经丢失。工具一直在改变,但是提交信息一直存在。
Jira 和 Stash 对获取一些背景信息非常有帮助,用于理解决定修改时来来回回都发生了什么。 我们的提交信息会捕获来来回回的最终结果,即使链接无效了,提交信息也应该能独立存在。
我们知道形式
我们的信息会被各种不同的工具和不同的场景阅读--pull request, IDEs, git log
命令行可能展示精简的或全部信息。关于书写的形式需要了解这些重要信息:
- 1.提交信息有一个 主题行。 一条提交信息的第一行的前50个字符会被大多数工具特殊对待。在很多情况下,当通过
git log
输出进行阅读的时候,如果没有其他操作,那么就只能看到这50个字符。所以尽可能把你的提交信息概括到50个字符以内。如果可能,将 JIRA 的 issue ID 也包括在里面。 - 2.多数工具希望在主题行和消息体之间有一个空白行。
- 3.提交信息常常是通过命令行工具阅读,通常在左边还会有分支线(“铁路轨道”)。按行也写是很有好处的,一般每行都控制在72个字符左右,肯定不要超过78个字符(为“轨道”留出空间)。
不跨界
我们在为我们的信息选择形式的时候非常谨慎
- 架构和广义决定写到设计文档中
- 当需要解释代码做什么以及为什么的时候,写到代码注释中。
- 对于代码改变,我们写到提交信息中(你懂的!)。
- 最后,对于任何代码评审者很关心,但是挖坟者不会关心的信息,写到** pull request 的评论**中。
注意:鼓励以上几条相互引用 !
我们犯过的错误
正如你可能从上面可以看出的,很多想法都包含在一个很好的提交信息中,并且像大多数事情一样,需要练习和相互帮助来建立习惯。 这里有一些我们常见的错误。
模糊的手势
我们有时会写下这样的信息 “Fix <bug>” 或者 “Adds <functionality>”。有时这些信息并没有描述这个 bug 或者是 functionality。这些仅仅是提示和线索,剩下的工作需要读者去完成。
解释代码
描述新代码和解释变化是不一样的。有时我们通过描述代码的机制,来帮忙读者理解这段代码是如何运行的。然而代码注释应该更适合这种情况。有时我们尝试解释高层的设计或者架构,这些最好放在我们可以链接到的设计文档中。不管是哪一个层面的解释,都有比提交信息更合适的地方书写。
将变化翻译成字面意思
很不幸的是,这一点非常常见。匆忙之中,我们会写简单而直白的在信息中描述变化。“Deleted foo”,或者 “Rename bar to baz”。 这个意思是正确的,但是像这样的信息太字面了。他们缺少上背景,动机,和阅读者寻找的安全保障。
范例
过错
这里是我曾经写下的真的没有任何意义的提交信息。对不起,我的伙伴们!
commit ee46355557abe4ebc816f6cef9821e64c25b22f0
Author: Logan Johnson
Date: Wed Dec 9 17:49:35 2015 -0500
GET /account/status
diff --git a/common/services/src/main/java/com/squareup/server/account/AccountService.java b/common/services/src/main/java/com/squareup/server/account/AccountService.java
index da02d2f..20c7c43 100644
--- a/common/services/src/main/java/com/squareup/server/account/AccountService.java
+++ b/common/services/src/main/java/com/squareup/server/account/AccountService.java
@@ -48,11 +48,11 @@ public interface AccountService {
* @deprecated use {@link #status()}
*/
@Deprecated
- @POST("/1.0/account/status") //
+ @GET("/1.0/account/status") //
void status(SquareCallback<AccountStatusResponse> callback);
/** Queries the account status. */
- @POST("/1.0/account/status") //
+ @GET("/1.0/account/status") //
AccountStatusResponse status();
/** Saves all preferences to server. Any null values will be left unchanged on the server. */
你可以看到它没有提供任何变化的背景或动机,仅仅是描述了代码。我不知道为什么要将请求从 POST 修改为 GET。有 bug 吗?也许吧,但是 bug 是什么?是请求本身的问题?我在修复我们的 REST 库里面的一个 bug,现在可能已经修复了?这个变化应该在某一个时间点还原?使用 POST 具体的错误在哪里,为什么我使用 GET 就会好一些?这么多问题!到现在我也不知道答案。
好一点的
我想我在这里做的更好一点:
commit cb5f051c5ee95e795dedaf3a264062cf75d44b1e
Author: Logan Johnson
Date: Mon Aug 8 14:41:04 2016 -0400
don't register Transaction in multiple scopes
RA-14998
TransactionBundler used to implement both Bundler and PausesAndResumes.
In order to correct a lifecycle tangle (obvious from the dual
implementation), we separated those two concerns and made Transaction
implement PausesAndResumes instead. That change was made in this PR:
https://stash.corp.squareup.com/projects/ANDROID/repos/register/pull-requests/10689/overview
Unfortunately, a side effect of (or possibly, motivation for) having the
TransactionBundler implement PausesAndResumes is that we vend a
TransactionBundler instance to each caller. That instance would end up
getting registered with an Activity scope, which was safe since each
caller would get a new instance-- even though Activity scopes overlap, a
single PausesAndResumes instance would only ever be registed with a
single scope.
By making the Transaction implement PausesAndResumes, we created a
problem. Transaction is scoped to the Application, and outlives any
Activities. Since Activity lifecycles (and thus scopes) overlap, this
meant we would attempt to register the Transaction in an incoming
Activity scope (via PauseAndResumeRegistrar) while it was still
registered in the outgoing Activity's scope. This caused a pretty
popular crash.
The fix here is to return to vending a PausesAndResumes object to each
caller, so that a separate instance is registered with each Activity
scope, avoiding the dual-registration crash.
note: The attentive reader will see that it's not actually the
PausesAndResumes that gets registered with the Scope. It's another
Scoped object that wraps the PausesAndResumes and delegates its
equals/hashCode.
diff --git a/register/src/main/java/com/squareup/payment/Transaction.java b/register/src/main/java/com/squareup/payment/Transaction.java
index fd09de4..1121c5e 100644
这一次,我们有了更多答案。这里有一些历史介绍,包括介绍这个问题的链接,可以阅读更多信息。为了让读者理解,我详细描述了我修复的这个bug(至少我在8个月以后还能理解)。最后,有了这些背景支持,我简单的描述了我做了什么修改,以及为什么这个修改可以修复这个问题。今天来看这个对我就有用的多,我希望对其他人也有帮助。