我在路径覆盖小组的一家机器人初创公司工作,提交拉取请求后,我的代码得到了审查。

我的团队成员已经在团队中工作了一年多,对我的代码进行了一些注释,这些注释表明我所做的工作比我认为需要做的要多得多。不,我不是一个懒惰的开发人员。我喜欢优雅的代码,它具有良好的注释,变量名,缩进并能正确处理大小写。但是,我不同意他的组织类型。

我将提供一个示例:

我花了一天的时间编写测试用例,以对我所做的过渡发现算法进行更改。他曾建议我处理一个不太可能发生的晦涩案件-实际上,我不确定是否有可能发生。我编写的代码已经可以在我们所有的原始测试用例和发现的一些新用例中使用。我编写的代码已经通过了每晚运行的300多次仿真。但是,处理这种晦涩的案例将花费我13个小时,最好将其用于改善机器人的性能。需要明确的是,迄今为止我们一直使用的以前的算法也无法处理这种晦涩的情况,而且在生成的40k报告中,这种算法从未发生过。我们是一家初创公司,需要开发产品。

我以前从未进行过代码审查,并且不确定我是否过于争论。我应该安静下来,按照他的话做吗?我决定保持低调,即使做出很大的改变,我还是会做出改变。


评论

您确实意识到,单元测试部分是要在有人检查您的代码更改(以某种晦涩的方式破坏它)时捕获百万分之一的缺陷,对吗?单元测试不仅要确保您的代码现在正确,而且还要三年内其他人维护您编写的代码。

@Klik“真正的输入将永远不会像这样”这就是您错了。我遇到过太多“输入永远不会像这样”的情况,当它“像这样”时感到惊讶。在生产环境中,可能会发生各种奇怪的事情。不仅要考虑您的软件将如何工作,还要考虑它将如何失败?

@Snowman更重要的是,代码审查的部分目的是要弥补那些单元测试甚至随机测试/模糊测试都找不到的百万分之一的缺陷。
还值得记住的是,代码审查不仅可以解决问题,还可以帮助您学习如何做得更好,因此将其视为学习机会。
嘿@Klik,听起来这里的根本问题是您“害怕说出自己的想法”。永远不要生气,总是说出自己的想法-带微笑。您应该立即告诉那个家伙:“嗯,那至少要花我两天时间,这值得吗,让我们问一下老板吗?”其次,您应该说:“别忘了我们正在研究机器人技术的人,实际上左传感器不可能在右传感器的右边-让我们问老板我们要多少防物理死角案例覆盖”。你的问题是你的,你的问题是你需要把愤怒换成话题。

#1 楼


不太可能发生的晦涩案例-实际上,我不确定是否有可能发生


在代码中没有未经测试的行为非常重要。如果运行一段代码,例如每秒50次,大约每5.5小时运行时间就会发生百万分之一的机会。 (对于您而言,赔率似乎更低。)

您可以与经理(或工作单位的高级负责人)讨论优先事项。您将更好地了解是否优先考虑代码性能或防弹代码,而这种情况可能不太可能。您的审阅者也可能对优先级有偏见。与负责人交谈后,您会更轻松地(不同意)您的审稿人的建议,并且有一些需要参考的内容。

一位审稿人。如果您的代码仅由一位同事审阅,请要求知道该代码的其他人或一般的代码库进行查看。再次,第二种意见将帮助您更轻松地(不同意)审稿人的建议。同样的问题又一次又一次地出现。尝试找出更大的问题,并直接与审阅者讨论。问足够多的问题。当我开始练习代码审查时,这对我有很大帮助。

评论


@ 9000如果答案是“因为生活就是那样”,这也可能令人沮丧。例如,这是我最近必须经历的:“在选项卡上使用空格”。 “为什么?” “因为我们的风格指南这么说。” “为什么?” “我不知道;我没有写。” “哈,显然你错了,我是对的,我将用制表符留下我的代码!”然后,无论如何,提交后的钩子都会改变它,但是-如果您使用5种Whys技术,请直到获得明智的答案为止,而不是使其他人感到沮丧为止。

–莫妮卡基金的诉讼
17年2月5日在8:47

@QPaysTaxes:每个人都应该知道为什么制表符上的空格(或制表符上的空格)参数的原因:因为两者都是正确的,但是一致性更重要,所以选择一个,保持一致,不要浪费时间

– slebetman
17年2月5日在10:19

在代码中没有未经测试的行为可能非常重要。如果运行一段代码,例如每秒50次,大约每5.5小时运行时间就会发生百万分之一的机会。 (在您的情况下,几率似乎更低。)-什么?不会。除非您正在运行蒙特卡洛模拟,否则您的代码将包含一些随机元素。除非您特别告知计算机,否则计算机不会根据钟形曲线或标准偏差来运行软件。

–罗伯特·哈维(Robert Harvey)
17-2-6在19:35



@RobertHarvey:考虑类似竞赛条件之类的东西,这恰好是一个随机元素。在处理流数据时,还要考虑与数据有关的错误。虽然数据可能不是完全随机的,但是除非高度重复,否则字节的特定组合可能会一次又一次地出现。百万分之一=是由20位的特定组合触发的,如果处理视频流,将立即看到类似的错误;很少发生但定期发生的事情可能涉及例如40位

– 9000
17年2月6日在20:19

@RobertHarvey:很有可能!我只是想指出,在一段特定的调用下,有几段代码可能有1e-6几率(不是0而不是1)被破坏,这是指“极不可能发生的模糊情况”。尽管类似的错误通常在测试中不可见,但在生产中是可见的。

– 9000
17-2-6在20:29



#2 楼

我可以举个例子,说明发生事故绝不可能发生的极端情况。

开发Ariane 4时,横向加速度计的值经过缩放以适合16位带符号整数,并且由于加速度计的最大可能输出(按比例缩放)永远不会超过32767,最小值也永远不会低于-32768,因此“不需要范围检查的开销”。通常,应该在进行任何转换之前对所有输入进行范围检查,但是在这种情况下,这将试图捕获一个不可能的极端情况。

几年后,开发了Ariane 5,其代码用于横向加速度计的缩放比例在“经过验证”的情况下经过最少的测试即可重复使用。不幸的是,更大的火箭可能会期望更大的横向加速度,因此加速度计得以升级,并可能产生更大的64位浮点值。

这些更大的值“包裹”在转换代码中,请记住没有进行范围检查,并且1996年首次发射的结果并不理想。这使公司损失了数百万美元,并在程序中造成了重大中断。 ,极不可能的等):它们所编码的标准要求对所有外部输入值进行范围检查。如果对此进行了测试和处理,那么灾难可能已经避免了。

请注意,在Ariane 4中,这不是错误(因为对于所有可能的值,一切都正常运行)–这是未能遵循标准的问题。当代码在不同的场景中被重用时,它将灾难性地失败,而如果值的范围被裁剪,则可能会优雅地失败,并且为此存在测试用例可能会触发对值的审查。值得注意的是,虽然爆炸后编码人员和测试人员受到调查人员的一些批评,但大部分责任归咎于管理层,质量保证和领导。

澄清

虽然并非所有软件都对安全性至关重要,而且在失败时也不是那么引人注目,但我的目的是强调“不可能”的测试仍然具有价值。这是我所知道的最戏剧性的案例,但是机器人技术也会产生一些灾难性的结果。检查的地方。实施团队负责人或项目经理可能决定不尝试解决发现的任何失败,但应意识到存在任何不足。或者,如果测试太复杂或难以实施,则无论使用什么跟踪器和/或风险记录,都可能引起问题,以明确表明这是未经测试的情况,可能需要解决更改用途或防止不当使用之前。

评论


哦,天哪,答案就是这样。 OP正在处理具有物理影响的软件。标准已经提高。在他们再次查看代码之前,审阅者可能没有意识到这种“边缘情况”。只要有足够的时间,什么都不是边缘情况。您不想意外地将机器人的手臂摆动到某人的头部(不是我知道该代码涉及到机器人的手臂)。

– Greg Burghardt
17年2月5日在13:07

Greg&Steve,这是一个很好的例子,但这是一个bug的例子。这是一个显而易见的错误。从字面上看是“一个错误”,除以零。当您研究机器人算法时,您要考虑一个物理上可能的和非物理上可能的概念是一个中心思想。 (如果您愿意,请使用当前的设备在物理上还不可行)。正在此处讨论的两个开发人员之间的困惑是由于他们(非常令人惊讶)对这种范例并不满意。如果更高级的帅哥没有嵌入该范式,他们的整个团队就会遇到严重的问题。

–法蒂
17年5月5日在13:25

我认为有一些现实的例子可以证明为什么要涵盖边缘情况,但这不是其中之一。引用的示例是为任务使用错误的库的情况。香蕉的代码不应盲目地用在橙色上。这是在橘子上使用香蕉代码的人的错,而不是为无法处理香蕉的橘子编写代码的人的错。 (由于存在一家大型高科技公司,因此我不得不将Apple换成Banana。)

–原产地
17年2月5日在13:35

@JoeBlow一个错误,是的,但是是一个可以预防的错误,可能已经被其他测试用例和/或代码审查所捕获。上帝只知道某个时候有人在说“你们知道,我认为我们应该验证这个范围...”,而其他人会说“别担心...如果不可能的情况?”如果错误不足以证明我们的逻辑有很多不足之处,那么我不知道该说些什么...

– code_dredd
17年2月5日14:00



我要说明的一点是,您不应忽略从未发生,极不可能发生的测试用例,等等,它们所编码的标准要求对所有外部输入值进行范围检查。如果对此进行了测试和处理,那么灾难可能已经避免了。请注意,在Ariane 3中,这并不是错误,而是未能遵循标准。当代码在不同的情况下重用时,如果灾难性地失败,而如果值的范围已被裁剪,则很可能会正常失败。

–史蒂夫·巴恩斯(Steve Barnes)
17年2月5日在15:00

#3 楼

由于以前没有处理过,因此超出了您的工作范围。您或您的同事可以询问您的经理,是否值得为解决这种情况而努力。

评论


我认为这是关键点-尽管涵盖额外的情况确实是有用且有效的改进,但出于各种原因,没有必要将其浪费到现有的PR中。

– DeadMG
17年2月5日在10:48

之前是否处理过它也是无关紧要的IMO,它根本不在事先的任务描述中。

–Jasper N. Brouwer
17年2月5日在18:39

为了回应这种观点,提出评论意见的一个很好的答案可以是“我会出示机票,这是很好的一点。”那承认,“事情”需要完成,同时还可以使其与需要完成的所有其他工作一起排在优先位置。

–爱德
17年2月6日在22:18

不能更不同意。由于我们不知道具体细节,因此在该功能的初始实施过程中可能未曾提出过这个问题,这可能很容易,因为这可能是迄今为止没有必要的事情,这表明它到目前为止是非常幸运的。修改此算法的工作方式的代码审查(可能会增加这种极端情况的可能性)正是提出潜在问题的时间。是否超出范围应在对此进行适当评估之后确定,而不是在没有上下文的情况下立即决定。

–马修·雷德(Matthew Read)
17年2月6日在23:38

这是可怕的建议!由于以前没有处理过,因此超出了您的工作范围,不足以将每个错误报告都标记为wontfix,前提是您的规范很糟糕,因此即使没有考虑,也没有考虑到极端情况有适当的规格。

–Filip Haglund
17年2月7日14:30在

#4 楼

使用复杂的算法,很难证明您已经想到了现实世界中出现的每个测试用例。当您有意将一个测试用例弄坏了,因为它不会在现实世界中出现时,您可能会将其他您尚未想到的测试用例弄坏。经常发生的情况是,当您处理其他测试用例时,您的算法必定变得更加通用,因此您找到了简化方法并使之对所有测试用例都更可靠的方法。这样可以节省您在进行维护和故障排除时所花费的时间。那是因为您的算法可能不会改变,但是当没人记得这个未处理的用例时,它的输入可能会在几年后改变。这就是为什么经验丰富的开发人员在刚想到时会处理这类事情。如果不这样做,它稍后会再次咬住您。

评论


您在第二段中提出了非常好的观点。我以前曾经经历过,但是说清楚是很有帮助的。

– Klik
17年2月5日在17:14

第三种情况=宾果游戏。我主要使用遗留代码工作,在某些项目中,有80%的工作只是在固定假设的地方。我原样喜欢这个答案,但我想补充一点,有时可以通过设置准确且详细的错误消息来解决优美的故障,从而覆盖这种不可能的情况,尤其是在时间紧迫时。

–犹他那
17年2月6日在17:52

我喜欢记录以下异常:“ [错误描述]根据[签名人的签名]签名的规范[版本],这不会发生。

– Peter Wone
17年2月13日在5:53

但是,由于之前没有测试用例,因此该代码(假设规格将替代以前的规范)不必在该迭代中适合新的测试用例。并且,如果还没有针对该极端情况的测试,则应该做一个新的工单/任务,而不是代码审查反馈imo。

–kb。
17年2月13日在11:25

#5 楼

这不是技术问题,而是业务策略决策。您会注意到建议的修复程序是针对非常模糊的情况,几乎不会发生。如果您正在编程玩具,或者正在编程说医疗设备或武装无人驾驶飞机,那么这将有很大的不同。罕见故障的后果将大不相同。

在进行代码审查时,在决定投资多少用于处理罕见情况时,应该对业务优先级有基本的了解。如果您在解释业务重点时与同事不同意,则可能需要与业务方面的人进行讨论。

评论


确实,请问其他人是否值得报道。我至少要编写测试用例,然后用注释将其标记为“已跳过”,并指出其他人做出了不值得实施的决定。 CYA

– Sean McSomething
17年2月6日在19:33

是的,每当代码审查中出现一些规模较大,范围较广但不紧急的事情时,我们都会在跟踪器中为其创建一个问题,然后将其与我们需要完成的其他其他事项优先处理。有时这意味着任务被放逐到优先级列表的远端,但是如果是这种情况,那么它实际上并不重要。

– Tacroy
17年2月6日在20:03

“这不是赔率,而是赌注!”

–user1068
17年2月7日在19:38

这是最好的答案。问题实际上是关于设置优先级和管理风险。

–wrschneider
17年2月8日在19:51



我同意这是商业决定。创建一张带有您估计需要修复的时间的票证,将其分配给老板(由指挥官负责分配您的时间),并要求为其分配优先级(视情况而定)也许)

– Matija Nalis
17年2月13日在12:17

#6 楼

代码审查不仅仅涉及代码正确性。实际上,这远不及利益共享,知识共享以及建立团队风格/设计共识的过程缓慢而稳定的过程。 “正确”常常是值得商,的,每个人对此都有自己的看法。以我的经验,有限的时间和精力也会使代码审查变得非常不一致-取决于不同的开发人员以及同一开发人员在不同的时间,是否可能会遇到相同的问题。审阅者的思维方式是“将如何改进此代码?”经常会提出一些建议,建议他们自然不会增加自己的工作。 。有时他们会要求我做出我略微不同意或认为不重要的更改。但是,我仍然进行这些更改。只有当我认为最终结果会导致产品质量下降,时间成本非常高或“客观”的观点可以客观化时(例如,要求进行变更的情况下),我才会与变更进行斗争不能使用我们使用的语言进行工作,或者基准测试表明所声称的性能提升不是一个)。

因此,我建议您谨慎选择战斗。为您认为不必要的测试用例编写两天,可能不值得花时间/精力进行。如果您使用的是诸如GitHub拉取请求之类的审阅工具,则可能会在此处评论您认为的成本/收益,以表明您的反对意见,但仍同意继续工作。这算是一次平缓的回推,所以审阅者知道他们正在达到极限,更重要的是要包括您的理由,这样,如果陷入僵局,这种情况就可能升级。您想要避免加剧的书面分歧-您不想因工作差异而引起Internet论坛风格的争论-因此,首先讨论问题并记录讨论结果的合理摘要可能会有所帮助。仍然同意对工作的需求表示不同意见(完全有可能进行简短的友好讨论将决定您双方的意愿)。计划会议等。请尽量保持中立,例如“在代码审查中,开发人员A确定了这个额外的测试用例,花了额外的两天时间来编写,团队是否认为以此为代价证明额外的覆盖范围是合理的?” -如果您实际进行此工作,则此方法会更好,因为它会给您带来积极的印象;您已经完成工作,只想轮询团队以规避风险和开发功能。

评论


“尽可能保持中立。”相当。我将比尼尔斯走得更远,并建议,正如他们所说,您需要交换“愤怒进行交谈”。只需立即(例如,在当前的特定示例中)公开地说:“史蒂夫,您的转角盒在当前的机械设计中是非物理的,您不认为伙计吗?让我们首先确定它是否非物理,甚至值得花两天时间。”

–法蒂
17-2-5在13:27

一个关键的观察是“如果您正在使用审阅工具”。 IME,这些工具足以保留评论记录,但实际工作需要与讨论面对面地完成。这是双向信息流的最有效方法。如果您不同意评论,请亲自进行建设性的意见分歧,然后将商定的结果输入工具中。

– Toby Speight
17年2月6日在10:56



@TobySpeight:我同意并试图将其纳入答案。

–尼尔·斯莱特(Neil Slater)
17年2月6日在11:24

2个小时是我不会战斗的。 2天,我将无法完成本周要完成的所有其他任务。

–马修·雷德(Matthew Read)
17年2月6日在23:43

#7 楼

我建议您至少断言晦涩的案子。这样,将来的开发人员不仅会看到您积极地对此案做出决定,而且还应具有良好的故障处理能力(应该已经就位),这也会引起意外。

然后,断言失败的测试用例。这样,行为就可以得到更好的记录,并可以在单元测试中显示出来。

显然,该答案假设您对“甚至不可能的可能性”的判断是正确的,我们无法判断。但是,如果是这样,并且您的同事同意,那么对事件进行明确的声明应该是对你们俩都令人满意的解决方案。

评论


完全同意。如果在任何情况下您的代码都无法处理,请尽早检查输入,然后在此输入失败。 “如果我们执行X程序会崩溃”总是比“如果我们执行X机器人会杀死人”更好。

–约瑟夫说恢复莫妮卡
17年2月6日在14:08

好建议。好的代码是经过证明永远不会失败的代码,但是,即使它莫名其妙地失败了,它也会以一种可以修复的明确方式失败。可能不会出错的代码,但是如果确实出错,则证明无法获取或修复该代码不是那么好...

–leftaround关于
17年2月6日在17:41



这是我要发布的完全相同的内容。审阅者指出了可能的失败,如何处理是另一个问题。

–SH-
17年2月9日在15:51

哦,不,不要在您的代码中声明。如果未编译断言,则没人会看到它。如果将assert编译进来,那么您的机器人将崩溃。我已经看到了不止一种情况,其中在生产代码中断言“永远无法发生的事情”的断言不仅触发了该系统,而且还破坏了所有依赖该系统的系统。

–汤姆·坦纳(Tom Tanner)
17年2月13日在10:28

@TomTanner“具有良好的故障处理能力,应该已经就位”。我假设代码已经可以在这里处理失败的断言。由于安全故障策略应该是任何物理系统的一部分,所以这可能不是很大。

–WorldSEnder
17年2月13日在15:27

#8 楼

由于您似乎很陌生,因此您只能做一件事-向团队负责人(或项目负责人)咨询。 13小时是一个商业决定;对于一些公司/团队,很多;对于一些,什么都没有。这不是您的决定,现在还没有。

如果线索说“发现那个案子”,很好;如果他说“ nah,搞砸了”,那很好-他的决定,他的责任。一次或两次将任务退还给您是完全正常的。

#9 楼

我认为我没有看到过实物,尽管它是@SteveBarnes提出的。 >在某些字段中,失败是网页上的错误。 PC蓝屏并重新启动。

在其他领域,这是生死攸关的事-自驾车锁死了。医疗起搏器停止工作。还是按照史蒂夫的回答:东西爆炸会导致数百万美元的损失。

在这些极端情况中存在巨大差异。

是否需要13个小时才能覆盖“失败”的价值最终不应该由您自己决定。它应该取决于管理层和所有者。他们应该对整体情况有一种感觉。您的机器人会减速还是停止?性能下降?还是机器人故障会造成金钱损失?失去生命?

该问题的答案应促使答案为“值得花13个小时的公司时间”。注意:我说公司时间。他们付账单,最终决定什么值得。您的管理部门应拥有最终决定权。

评论


此外,即使是晦涩难解的已知缺陷也不会带来什么责任后果?

–chux-恢复莫妮卡
17年2月11日在2:39

#10 楼

无论您是初级开发人员还是高级开发人员,甚至是首席执行官,处理分歧的最佳方式都是相同的。

像哥伦布一样。

如果您从未看过任何哥伦布(Columbo),那是一场非常棒的表演。哥伦布(Columbo)是个非常谦虚的人物-大多数人都认为他有点发疯,不值得担心。但是,通过显得谦虚,只要求人们解释一下他就能得到他的男人。通常,您想问自己和他人的问题,以确保您做出正确的选择。不是从“我是对的,你错了”的立场出发,而是从诚实的发现立场出发。或者至少要尽力而为。

在您的情况下,您有两个想法,但是它们的基本目标是相同的:使代码变得更好。

重新给人一种印象,那就是跳过代码覆盖范围而转向可能开发的其他功能(这是不可能的)是最好的方法。小心角落的情况会更有价值。

他们看到了什么,您看不到?您看到他们看不到的东西了吗?作为初级开发人员,您实际上处于有利位置,因为您自然应该提出问题。在另一个答案中,有人提到极端情况的可能性。因此,您可以从以下内容开始:“帮助我理解-我在X,Y和Z的印象中-我缺少什么?为什么小部件会碎屑?我印象是在极端情况下它会变硬。摇摇杆实际上是ANZA刷子的标志吗?”

当您对自己的假设和发现提出疑问时,您将挖掘,发现偏见并最终弄清楚正确的做法是什么。
首先假设您团队中的每个人都是完全理性的,就像您一样,他们在团队和产品方面都拥有最大利益。如果他们做的事情没有意义,那么您需要弄清楚您不知道他们做什么,或者您知道他们没有做什么。

#11 楼

也许和负责安排工作优先级的人交谈?在启动时可以是CTO或产品所有者。他可以帮助查找是否需要这项额外工作以及原因。您也可以在日常站立时带来烦恼(如果有的话)。

如果没有明确的计划工作职责(例如产品负责人),请尝试与您周围的人交谈。后来可能成为每个人都将产品推向相反方向的问题。

#12 楼

13小时没什么大不了的,我会做的。请记住,您为此得到报酬。只需将其归纳为“工作保障”即可。另外,最好在团队中保持良好的业力。现在,如果这需要您一个星期或更长的时间,那么您可以邀请您的经理来询问他,这是否是您最好的时间利用方式,尤其是您是否不同意。

但是,听起来您需要在团队中发挥作用。这是您获得杠杆作用的方式:寻求宽恕,无需征求许可。在您认为合适的情况下向程序添加内容(在课程范围内,即确保它完全解决了老板想要的问题。),然后将事实告诉经理或您的同事。不要问他们:“如果添加功能X,可以吗?”相反,只需在程序中添加您个人想要的功能。如果他们对新功能感到不满或不同意,可以将其删除。如果他们喜欢它,请保留它。

此外,每当他们要求您做某事时,都要“加倍努力”,并添加很多他们忘记提及的内容或比他们说的更好的内容。但是,不要问他们是否可以继续努力。只要做到这一点,并在完成后随便告诉他们。您正在做的是对他们进行培训。

发生的事情是,您的经理将您钉为“志趣相投的人”,并将开始信任您,您的同事将开始看到您作为领导,您将开始拥有该程序。然后,当像您提到的事情将来发生时,您会拥有更多发言权,因为您本质上是团队的明星,如果您不同意,他们会退缩。

评论


尽管我全心全意地让程序员积极进取,而不仅仅是“从高处接受订单”,但是您提出的方式在专业上是不负责任的,而且不道德。您基本上是说,OP应该花费雇主的时间和金钱,而不是使用请求的功能,而是使用“个人需要”的功能,然后花费雇主的时间和金钱,删除这些功能。这甚至不考虑潜在的缺陷增加或其他开发人员花费时间进行代码审查/维护。我会以您描述的态度解雇开发人员,特别是初级人员。

–德里克·埃尔金斯离开东南
17年2月5日在6:59

好吧,你在某种程度上是对的。尤其是如果工程师自己跳槽而对客户的愿景不敏感。但是,请不要破坏我完全说的话-我只是说要“努力”。因此,这意味着您接受老板所说的话,然后继续进行下去。在软件中,这就是看起来很琐碎的软件和看起来是由专业人员制作的软件之间的区别。我知道许多开发人员即使被告知完全是垃圾,也“完全按照他们的意思”。那些开发人员从来不算什么。

–user19718
17-2-5在7:11



有负责任的方式来做您描述的事情。很多时候,需求会留下很多空间,您可以根据自己的判断来获得更完美的结果(在包括维护工作在内的工作量之间取得平衡)是一件好事。主动指出并修复错误通常是可以的。花您自己的时间来制作您认为对公司有利的功能原型,并将其呈现给团队以供可能使用也可以。您的“个人需求”是无关紧要的,在您获得报酬时,重点应该放在企业的利益上。

–德里克·埃尔金斯离开东南
17年2月5日在7:12

请参阅我的第二条评论,以了解我如何表达我相信您想要表达的想法。正如我说的,我的问题更多是与您的陈述方式有关。开发人员需要自豪地知道他们可以做出有意义的决定,但是要谦虚地知道他们(通常)对业务目标或优先事项没有全面了解。更多的高级开发人员不太可能做出错误的决定,而更有可能知道业务目标是什么以及如何朝着目标迈进。

–德里克·埃尔金斯离开东南
17年2月5日在7:15

还请注意,我的评论适用于那些希望升任领导或顾问级别的人员。公司特地聘请我征求意见。

–user19718
17年2月6日在17:32

#13 楼

代码审查有几个目的。您显然知道的一个问题是:“此代码适合目标吗?”换句话说,它在功能上是否正确?是否经过充分测试;对非显而易见的部分进行了适当的评论;它是否符合项目的约定?

代码审查的另一部分是共享有关系统的知识。对于作者和审阅者而言,这都是一次了解更改后的代码及其与系统其余部分的交互方式的机会。

第三方面是它可以对以前存在的问题进行回顾进行任何更改。通常,在检查其他人的更改时,我会发现上次迭代中遗漏的内容(通常是我自己的内容)。这样的声明“这是使它变得比以前更强大的机会”,这不是批评,也不要将其作为一个整体。提交之前必须毫发无损地传递代码的网关或障碍,但主要是作为对特定功能领域进行结构化讨论的机会。这是信息共享最有效的场合之一。 (这是在团队中而不是总是由同一位审阅者共享审阅的充分理由。)

如果您将代码审阅视为对抗活动,那是一种自我实现的预言。相反,如果您将它们视为工作中最具协作性的部分,则它们将刺激您的产品以及您的合作方式不断得到改进。如果可以使评论清楚地了解其建议的相对优先顺序,则将有帮助-例如,“在这里我想发表有用的评论”与“如果x永远是负面的,这会中断”之间有很大的区别。

上面做了很多一般性陈述后,这对您的具体情况有何影响?我希望现在很明显,我的建议是对公开的问题做出回应,并商讨哪种方法最有价值。对于您建议进行额外测试的示例案例,则类似“是的,我们可以对此进行测试;我估计需要

#14 楼

代码总是可以变得更好。

如果您正在进行代码审查,但是看不到任何可能更好的东西或单元测试可能会发现错误,那不是完美的代码,但没有完成工作的审稿人。是否选择提及改进是个人选择。但是几乎在您的团队进行代码审查的任何时候,都应该有人会发现某些事情可能会变得更好,或者每个人都可能浪费了他们的时间。给你的团队。如果您的更改解决了该问题或在没有更改的情况下增加了足够的价值,以致您的团队接受了这些更改,则将其合并,并将其评论记录到待办事项列表中,以供以后解决。如果团队发现您的更改增加的风险或复杂性超过价值,那么您必须相应地解决问题。

请记住,任何代码至少还有一个可以测试的极端情况,并且可以用于至少还有一个重构。这就是为什么最好在每个人都同时查看相同代码的情况下以小组的形式进行代码审查的原因。这样一来,每个人都可以就所审查的代码是否可以接受(就这样)达成共识,并增加足够的价值以合并到社区基础中,或者是否必须做某些事情才能合并足够的价值。

由于您提出了这个问题,所以我假设您实际上并不是在进行“代码审查”,而是创建了一个拉取请求或其他提交机制,以便其他人以不确定的方式发表评论。因此,现在您遇到了管理问题和完成的定义。我猜您的管理层是举棋不定的人,实际上并不了解代码审查的过程和目的,并且可能没有“完成的定义”(DOD)。因为如果他们这样做,国防部将明确回答这个问题,而您不必来这里问。

您如何解决?好吧,请您的经理给您一个DOD,并告诉您是否必须始终实施所有注释。如果相关人员是您的经理,那么答案不言而喻。

#15 楼

这个问题与防御性编程的优点,极端情况的危险或物理产品中的错误的灾难性风险无关。实际上,它甚至根本不是关于软件工程的。

真正要解决的是,当大三学生不同意或欣赏它时,小三级从业者如何处理高级从业者的指令。 >
作为一名初级开发人员,需要了解两点。首先,这意味着尽管您可能是对的,而他是错的,但在概率平衡上,这不太可能。如果您的同事提出您看不到它的价值的建议,则需要认真考虑一下您的经验不足以了解它的可能性。

我要感谢的第二件事是,您的高级合伙人之所以被称为是因为他负有更多责任。如果大三学生破坏了重要的事物,那么按照指示进行操作就不会有麻烦。但是,如果高层允许他们打破它-例如,通过不在代码审查中提出问题-他们将很麻烦。您遵守那些由商业信托机构领导的项目的指示。当您有充分的理由重视他们的经历时,您通常无法屈服于老年人吗?您是否打算不听不懂的指示?您是否认为世界应该停止,直到您被说服?这些价值观与团队合作是不相容的。

最后一点。回想六个月前您编写的项目。想想你在大学写的项目。看看它们现在看起来有多糟糕-所有错误和颠倒的设计,盲点和误导的抽象?如果我告诉您,六个月后您会发现您今天所做的工作中存在同样的缺陷该怎么办?这是否有助于说明您目前的方法中有多少盲点?因为那是经验带来的差异。

#16 楼


经常对我的代码进行注释,建议我做很多不必要的工作。决定什么是必要的。实施管理层或(在此情况下,您的审阅者)认为必要的事情是您的工作。如果您过分或过于不同意必要的内容,您很可能会失业。因此,习惯于此并与之融洽相处是您专业精神的一部分。


他建议我处理一个晦涩难懂的案件,这种案件极不可能发生


这里还有其他一些很好的答案,这些答案表明即使仍然存在非错误(即,证明是永远不会失败的东西),有时也应该进行重新处理。 (例如,在构建产品的未来安全性,遵循标准等情况下)。出色的开发人员的部分职责是尽可能地确信自己的代码在每次可能的情况下以及将来都会健壮以及不仅在大多数情况下在当前条件下经过测试的情况下都经过验证的

#17 楼

对代码审查者的建议,以提高您的代码审查的业务实用性(OP则应提出这样的更改): “关键” /“必须” /“可选” /“建议的改进” /“很高兴” /“我在沉思”。

如果这看起来过于CDO /肛门/复杂,请至少使用2个级别:“必须修正才能通过审核并允许您合并更改” /“所有其他”。


处理似乎不太重要的代码审查建议的建议: ,跟踪建议
,如果您的流程允许对Fisheye或电子邮件评论之类的评论做出回应,则将票证编号作为对代码审阅项目的评论。如果该项目属于“必须修正或将不被合并/发布”类型。


如果答案为“是”,但您不同意,请负责此操作的人员项目管理人员(项目经理,您的经理等)进行决策-诚实完整地表达分歧。这不是关于哪个人是“正确的”,而是关于什么对项目更有利,因此PM /经理的工作。开发请求。


如果升级后决定紧急,请将其视为任何紧急开发请求。取消对其他工作的优先级并为此进行工作。
否则,请根据为其分配的优先级及其ROI(根据其他答案中的说明,根据您的业务领域而有所不同)进行处理。

#18 楼

您不应将此升级到管理层。

在大多数公司中,管理人员总是选择不编写额外的测试,而不是花时间少量地提高代码质量,而不会浪费时间进行重构。

在很多情况下,代码质量取决于开发团队中的不成文规则以及程序员所付出的额外努力。

您是初级开发人员,这是您的第一次代码审查。您必须接受建议并进行工作。如果您先了解并尊重他们一段时间,就只能改善团队中的工作流程和规则,以便您了解它们为什么会存在。否则,您将成为不遵守规则并成为团队孤独者的新人。

您是团队的新手,请按照您一段时间得到的建议,找出它们为什么存在,不要在Scrum会议中提出您的第一条建议。一段时间后,您会发现真正的业务优先事项毫无疑问(而且可能不是管理人员面对面告诉您的内容)。

评论


不幸的是,当您起步不错时,您的建议最终会变得很糟糕。

–约书亚
17年2月8日在16:30

#19 楼

编写满足领导/管理要求的代码非常重要。任何其他细节都只是一个“不错”。如果您是该领域的专家(请阅读:“如果您不是初级开发人员”),那么您就“有资格”解决您在此过程中发现的小问题,而无需每次都与您的领导联系。

如果您认为某事不对且您是该领域的专家,那么您就很有可能是对的。

以下一些对您有用的陈述: />
我被要求做X,代码审查员建议也做Y,我应该这样做还是继续研究更重要的东西?至少有一个测试用例可以捕获该行为,以便我可以对其进行测试?我相信不会调用代码。
也许我们不应该首先为未发现的案例开发一个安全的后备吗?因此,我们会尽早发现并解决问题,以便我们专注于更重要的事情。
好,在我实施Y的同时,您是否愿意为它编写一些测试用例,以便一次完成工作到底是什么?
我被要求做X,我认为我可以做Y,除非有其他优先事项。下次为什么不提交功能请求,而不是将其作为对我的代码的评论?至少我们可以在实施该功能之前听取其他团队成员对该功能的进一步意见(通常,任何重要内容都应该是一项功能,并且应由一个以上的人员来处理;通常,代码审查应与审查代码和解决方案方法有关;其余是错误修复和功能)。

您是否真的认为审稿人的态度正在损害项目,或者您认为他大多数时候是对的(除非有时他在评估中犯了小错误,夸大了吗??

对我来说,听起来更像是他正在编写不属于代码审查的内容,这是一种不好的做法,因为它使每个人都很难跟踪内容。但是,我不知道他还发表了哪些其他评论,因此我无法在其他情况下发表任何意见。

一般来说,请尽量避免以下两种情况:


您没有按照要求进行操作
使您的审阅者感到愚蠢

如果他实际上正在审阅您的代码,那是因为管理层比您更信任他。

#20 楼

如果在代码审查期间对范围存在分歧,请:


记录该代码实际涵盖的范围。没有人喜欢讨厌的惊喜。
意识到范围是一项商业决定。在您开始使用某个功能时,范围应该已经知道了,但是如果不是这样,您以后总是可以要求澄清。

如果代码审查员是制定业务决策的人,则他可以随时更改范围,甚至在代码审查期间也可以,但是他并不是这样做的。
/>

评论


这似乎没有提供任何实质性的东西,在之前的20个答案中提出和解释过

– gna
17年2月11日在5:54

#21 楼

如果无法证明极端情况是不可能的,则必须假设它是可能的。如果有可能,那么不可避免的是它最终会发生,而不是早于以后。如果未在测试中出现过极端情况,则可能暗示测试覆盖范围不完整。


接受反馈。
在对代码进行任何更改之前,尽力为边缘情况构建测试,并查看是否会导致测试失败(证明问题存在)。如果不可能创建这样的测试用例并导致测试失败,那么您也许可以得出结论,边缘案例实际上是不可能的(尽管我会犹豫得出这样的结论)。
如果可能会导致测试失败,请应用适当的代码更改以通过测试。

为了产品的优良性,您可能希望能够生产边缘盒并引发失败,以便可以应用此修复程序,并有信心避免了潜在的问题。

代码审查的全部重点是要更加注意代码。我们每个人都不能避免错误或疏忽。多次查看一段代码而没有注意到明显的错误,这是很常见的,新的眼睛可以立即看到它。

正如您所说,您实现了一个新的代码算法。根据新算法的行为或对其前任的观察得出结论,这是不明智的。

评论


这似乎没有提供任何实质性的东西,在先前的21个答案中提出和解释过

– gna
17年2月13日在7:39

#22 楼

有些代码审阅者知道他们在做什么,如果他们说需要更改某些东西,那么就需要更改,而当他们说需要测试某些东西时,就需要对其进行测试。

有些代码审阅者需要通过为他人创建无用的工作来证明自己的存在。

哪个是您要决定的,如何处理第二种则更多是workplace.stackexchange的问题。

如果您使用Scrum,那么问题是您的工作是否完成了应做的事情(显然是这样做的),并且您可以在积压的待办事项中处理极其罕见甚至不可能的情况它会被优先考虑,如果进入冲刺阶段,那么您的审阅者可以随意取用它并进行13个小时的工作。如果您执行作业X,并且因为您执行作业X而意识到作业Y也需要执行,那么作业Y不会成为作业X的一部分,它是它自己的独立作业。

评论


这太含糊且令人激动,无法提供有用的建议。

–罗伯特·哈维(Robert Harvey)
17年2月6日在19:44

关于SCRUM的评论是完全正确的,突出显示了待办事项中的新任务。删除答案的粗鲁开头,您将获得积极的成绩。

– xmedeko
17年2月9日在19:17