Re: [閒聊] Code Review 意見不合

看板Soft_Job作者 (東周小星星)時間10年前 (2015/02/18 17:29), 編輯推噓53(530181)
留言234則, 25人參與, 最新討論串7/12 (看更多)
code review的問題, 想到以前曾經遇過一件事, 我進到某間公司的時候, 還在了解這公司的coding standard, 因為規定很多,所以一開始還沒有把所有的規定都背起來, 我把程式寫好後,就交給同事review, 同事就開始罵我: 「你是沒看文件嗎?這地方要空1行,為什麼要空2行?」 「為什麼你刮號要寫成 if(){ } 而不是 if() { } 這種寫法我沒看過,亂寫」 「你寫什麼,我都看不懂...」 我心裡就覺得很OOXX... 這個人的口氣真的很差, 後來再熟一點的時候, 也發現這位同事功力其實不如我, 而且最讓我不滿的是, 這位同事他自己也不是每個code都有符合規定的coding standard, 他懶的時候他也是沒照著standard來寫, 其實我非常認同要有code review這件事, 但是我給人code review只是想要去符合coding standard, 讓我的code,還有大家的code可以互相維護, 而不是要和同事比賽, 程式碼本身好不好, 這其實是很主觀的事, 但也許是人性問題吧, 發現有些人是抱著批評別人的心態在code review, 講出多餘的負面字眼也就算了, 還去要求別人做出coding standard以外的改變(私人要求), 也有人是完全不鳥coding standard, 被別人要求code review的時候, 就一直吵不肯改,這樣的人我也遇過... 以上兩種人都是個性很硬,對自己程式很有自信的人, 就像你所說的,很愛面子的人,進而不給別人面子, 弄到最後,code review也就變成政治活動了, 不爽的人就把別人拉入戰局,開始搞小圈圈,搞造神,搞文革, code review這件事也就被搞爛了, 這是我的感慨囉, 我現在蠻怕和「覺得自己程式寫很好」的人一起工作, 不管他的程式是不是真的寫得很好, 就像在打LOL, 若有人老是罵來罵去玩自己的,就算他打得很好, 你也不會想和他同一隊,因為他會破壞團隊氣氛導致輸場, 寫程式還是要抱著謙虛的心在寫比較好, 技術如此的多,世界如此的大,沒有人十全十美樣樣精通, 越學,會覺得自己越渺小 ※ 引述《jackyu (孫權)》之銘言: : 小弟弟我覺得這篇思考的點和我不知道為何搭不太上 : 但是其實原原PO我覺得倒是問題不大 : 不過在面子勝過一切的華人圈, 你要了解一件事 : 就是code review change => 你輸給了這個reviewer => 頗多工程師的邏輯 : ※ 引述《yauhh (小y寶貝)》之銘言: : 從原文看到很有可能變量命名是公司的規定 : 代碼執行效率我比較好奇, 因為演算法和設計/系統/介面接口有很大的不同 : 有O(n)的做法可是不得已作成O(n^2)也是有(特別是code很老和需求太雜) : 我通常不會那麼佛去看這塊, 因為設計的review是架構師才有權力決定其對錯 : 自己review這個真的吃N倍的力又不討好 : 如果在多人合作的環境LOG開關是很重要的禮貌: : 因為LOG太雜是很傷害debug的精力, 而且還要修過版本控制裡的內容我才能好好的做事 : unit test是自己在上傳前就要搞定的事, 當然包括移除不必要的LOG : 若是日後整合測試, 當然要留開關 : : 我想你應該先判斷,自己的思維有沒有問題: : : 1. 你已經在記恨同事不幫你買帳,這種記恨變成下次有機會你就挑起爭端的 : : 藉口。 : 這邊我實在看不出來他在記恨耶, 我看到的是原原po再強調他要求被拒絕的事 : 只是這個"要求"若是公司規範, 那這有甚麼好檢討的? : 如果不是, 還是要看細節. 我只能說他是龜毛了些 : : 2. 你的說詞如果照字面來看,是強調因你的身份為 reviewer 而給品質 : : 帶來保障,而更甚於因你的 review 帶來品質的保障。 : 這邊實在看不出來, 原原po的ego有那麼高嗎? : 他只有說"我一定按公司的標準, 再加上以品質為前提的標準" : "若是覺得我的標準太龜毛, 請找別人" : 再依描述他們公司一個BUG的歸責似乎reviewer也會有或多或少的影響 : 既然和自己有關, 多要求一些有何相關? : : 3. 你的說詞如果照字面來看,同事可以不要找你 review 而是找別人幫他 : : review ,所以你根本不必對他警告什麼事嘛。 : : 4. 你覺得 code review 是一種連帶要求別人按照你喜歡的方式寫程式 : : 的手段嗎? : : review, revision, bug, coding style 等等,這些詞彙彼此是有不同內涵。 : : 我覺得你是錯的: : : 1. 記恨而會講出的那一番話,像是「我有我的原則,不要浪費我的時間」, : : 應該是上次發生爭端之後講的話,而不是下一次 review 之前你先樹立的 : : 障礙。你的錯是在你把你自己的人格擋在同事之前,同事不經過你就不得 : : 涉入工作,於是你的人格妨礙工作,妨礙公司的運作。 : 這論點有問題. : 若是人格特質是有利該團隊運作的, 為何算是樹立障礙? : 公事公辦我倒是不覺得有啥機掰之處 : 我的經驗妨礙團隊運作大多都是工程師莫名其妙的自尊心 : 有些人當reviewer挑人毛病很爽, 被review認為若是屈服就很不爽 : 擇善固執之後私下關係好的我真的沒看到幾個(華人..哀) : 好來好去review就等於虛設了 : : 2. 當你要強調你對現有的 code review 情況不滿的時候,應該客觀談那一件 : : 事情,而不應該把你自己的人格帶進來擺在上位。 : 我覺得他的論述很客觀啊, 阿就被reviewer一直盧 : 當然這是片面說詞, 不過就這片面說詞而言, 看不出來 : 而且我覺得這應該是就事論事後還被爐才爆發的 : : 3. 「管他去死」這樣的思維,在你的工作環境中,其實是代表你培養了可能 : : 讓你不適任的因素了。 : ㄟ, 不聽我的話BUG出來怪我囉? 當然管他去死啊 : 不適任在哪裡? 覺得我的review是屁然後真的出事了我還被以review的身分被high light : : 4. 人家可能只是說 review ,多看一眼,至於什寫法叫做問題,是要另外有 : : 開放討論的。你光是叫人一定要改,假如不改,你就不接受,那其實,一 : : 方面這不是開放討論,另一方面,由你指揮別人一定要改的那些東西, : : 如果有問題,應該是你要全權負責。 : 這個就是藏在細節裡的魔鬼了 : 很有可能review是他們公司dual programming的一環(我猜) : 因為到時候bug tracking可能只記錄誰review, : 但是review process沒有(有哪家公司會龜毛到這種地步嗎?XD) : 這時候被反咬不就很悶? : 給blabla原po, : 我很高興在台灣這種職場文化還有這種個性, 沒被磨掉 : 但是要記得, 若是你沒有100分的實力 : 別人只會記得你有"60分的龜毛", 並且質疑你對團隊的傷害和產出不成正比 : 我建議你以後在review用"建議"就好, 然後真的管他去死 : 並且增強實力, 然後默默的紀錄不聽你建議真的造成bug的事項 : 久了一攤開, credit有了講話也大聲 : 另外, 要旁敲側擊知道不接受是因為不想做還是不爽做 : 不想做是時間問題, 不爽做是面子問題 : 這兩點處理的手腕很不一樣 -- 寫程式是一種信仰, 寫得出來是一種藝術, 寫不出來是一種哲學。 -- ※ 發信站: 批踢踢實業坊(ptt.cc), 來自: 223.136.234.83 ※ 文章網址: https://www.ptt.cc/bbs/Soft_Job/M.1424251781.A.EB2.html

02/18 17:30, , 1F
像那種空幾行 {}的位置這種小事,讓IDE來處理不就好了
02/18 17:30, 1F

02/18 17:32, , 2F
那時候的standard,是細到規定在什麼狀況下要空幾行
02/18 17:32, 2F

02/18 17:33, , 3F
所以要人工判斷...因為不是一直固定空幾行
02/18 17:33, 3F

02/18 17:40, , 4F
只能說定standard的人太閒了當然這種東西有共識就好
02/18 17:40, 4F

02/18 17:43, , 5F
那standard我記得是厚厚一疊的,所以他們自己也沒遵守
02/18 17:43, 5F

02/18 17:43, , 6F
弄到最後,也只是某個人講什麼,說了就算
02/18 17:43, 6F

02/18 18:00, , 7F
這已經不叫訂standard了吧XD 根本就是自high
02/18 18:00, 7F

02/18 18:08, , 8F
以我的專案經驗來說,是匯入code style template,
02/18 18:08, 8F

02/18 18:09, , 9F
仍有文件定義,只是被簡化,同事間約定默契即可。
02/18 18:09, 9F

02/18 18:16, , 10F
你寫成if(){不空格我一定會發飆 這根本common sense
02/18 18:16, 10F

02/18 18:17, , 11F
啥都不空格的CODE 只會讓別人覺得你剛學程式的大一生嗎
02/18 18:17, 11F

02/18 18:19, , 12F
樓上,其實有短碼競賽啦,當然工作不能這樣玩。
02/18 18:19, 12F

02/18 19:02, , 13F
匯入Naming Guideline讓IDE處理 +1,誰知道誰的標準才是準的
02/18 19:02, 13F

02/18 19:03, , 14F
有統一的template,就算對方忘記空格也只要順手幫個忙就好
02/18 19:03, 14F

02/18 19:07, , 15F
code review的用意應該不是為了要看coding standard ...
02/18 19:07, 15F

02/18 19:09, , 16F
你寫成if() {空格我一定會發飆 這根本common sense
02/18 19:09, 16F

02/18 19:23, , 17F
統一用一個ide就好…另外code review不是在看這個的吧
02/18 19:23, 17F

02/18 19:24, , 18F
^ style template
02/18 19:24, 18F

02/18 19:56, , 19F
我也支持用ide處理就好,但那時候公司不用ide處理,變成
02/18 19:56, 19F

02/18 19:59, , 20F
很多事是人為去判斷,而且又規定得很細很富雜
02/18 19:59, 20F

02/18 20:01, , 21F
我知道code review更大的好處是了解別人的code和討論
02/18 20:01, 21F

02/18 20:03, , 22F
但很多時候爭議就從這邊來,有人就會惡性批評
02/18 20:03, 22F

02/18 20:06, , 23F
給的意見真的不錯的話,也就算了,有人給的意見反而比較
02/18 20:06, 23F

02/18 20:06, , 24F
爛,例如叫我不要用function,因為他不會看function
02/18 20:06, 24F

02/18 20:10, , 25F
某樓也示範了一個惡性批評的例子,還人身攻擊,這樣的人
02/18 20:10, 25F

02/18 20:11, , 26F
蠻多的,這並不會讓code的品質更好,只會讓氣氛更糟
02/18 20:11, 26F

02/18 20:40, , 27F
code review的重點不在此 只會著重在這些的真的很無聊
02/18 20:40, 27F

02/18 20:44, , 28F
笑了 舉得例子不好還說別人惡性批評 你怎不去想想自己為
02/18 20:44, 28F

02/18 20:44, , 29F
何不空格 寫code都寫給自己看懂就好 這種人更多
02/18 20:44, 29F

02/18 21:02, , 30F
那你們覺得code review的重點為何呢?來聊聊吧
02/18 21:02, 30F

02/18 21:36, , 31F
我也是覺得遇到不好相處的人合作比較難過....
02/18 21:36, 31F

02/18 21:36, , 32F
聊聊。
02/18 21:36, 32F

02/18 21:36, , 33F
我經歷的 code review 還有讀書心得。
02/18 21:36, 33F

02/18 21:36, , 34F
code review 的部分偏向抓版控上說明自己如何解這 b
02/18 21:36, 34F

02/18 21:36, , 35F
ug. EE丟問題過來你怎麼分析和開始動手解。前輩會判
02/18 21:36, 35F

02/18 21:36, , 36F
斷你是否真的解決。程式品質如何。和 performance
02/18 21:36, 36F

02/18 21:36, , 37F
等等
02/18 21:36, 37F

02/18 21:36, , 38F
讀書心得鳥了。研讀的 Effective C++主持人本身功力
02/18 21:36, 38F

02/18 21:36, , 39F
不夠,還不到第三十條就有人退出,也停止該讀書會。
02/18 21:36, 39F
還有 155 則推文
02/20 12:42, , 195F
也許看空格不花時間?但一來一回打斷工作節奏,成本其
02/20 12:42, 195F

02/20 12:42, , 196F
實不小
02/20 12:42, 196F

02/20 12:45, , 197F
最好是導入工具 並且確實執行 兩者兼顧 讓工程師回歸解
02/20 12:45, 197F

02/20 12:45, , 198F
題邏輯
02/20 12:45, 198F

02/20 16:55, , 199F
沒說看空格等於看架構啊 只是當你連規劃架構、調效能、
02/20 16:55, 199F

02/20 16:55, , 200F
實做功能的時間都沒有了 你還會執著在空多大格?都用工
02/20 16:55, 200F

02/20 16:55, , 201F
具排好、設定好不就好了?
02/20 16:55, 201F

02/21 03:30, , 202F
@xvid 那間公司不讓我導入tool 堅持用人工檢察空格
02/21 03:30, 202F

02/21 03:31, , 203F
看空格花不花時間? 你如果一個file只改一行 卻要去
02/21 03:31, 203F

02/21 03:53, , 204F
突然發現我用到別人帳號回文~Orz
02/21 03:53, 204F

02/21 11:35, , 205F
換回我自己的帳號重回一次XD
02/21 11:35, 205F

02/21 11:36, , 206F
我之前待過的公司 你一個file只改一行 卻要去看
02/21 11:36, 206F

02/21 11:36, , 207F
全部FILE的空格有沒有空錯~有時要修個小BUG要去看
02/21 11:36, 207F

02/21 11:37, , 208F
2~300行UP的空格.... 明明導入tool就搞定的事
02/21 11:37, 208F

02/21 11:38, , 209F
卻要弄的讓辨公室裡的人互相罵來罵去
02/21 11:38, 209F

02/21 12:12, , 210F
這也沒啥好吵的..比對軟體或version control有內建viwer
02/21 12:12, 210F

02/21 12:13, , 211F
"真的"只改一行 viewer一開人工一看就知道了
02/21 12:13, 211F

02/21 12:14, , 212F
不 他們要求的是 你改了一行 要去check那個檔案的
02/21 12:14, 212F

02/21 12:15, , 213F
全部coding style有沒有附合規格
02/21 12:15, 213F

02/21 12:15, , 214F
如果硬凹要說veiwer是不導入tool的一種 難不成公司都用
02/21 12:15, 214F

02/21 12:17, , 215F
改一行就全部重做? 反正公司錢多,腦袋不換就等被淘汰
02/21 12:17, 215F

02/21 12:37, , 216F
我也不知是不是其他人都一樣 至少我沒改會被帶我
02/21 12:37, 216F

02/21 12:38, , 217F
的人飆XD~導致後來我都只想改同一個檔案裡的內容XD
02/21 12:38, 217F

02/21 12:40, , 218F
除了被飆很不爽 但想想公司每個月花錢請我去改空格
02/21 12:40, 218F

02/21 12:40, , 219F
有一種很KUSO的感覺XD
02/21 12:40, 219F

02/21 15:43, , 220F
codereview最重要的不就是看你寫的演算法有沒有效率..
02/21 15:43, 220F

02/21 15:44, , 221F
很多人都本末倒置了 在ㄧ些無關緊要的細節挑毛病
02/21 15:44, 221F

02/21 18:53, , 222F
空格工程師表示:我會抓空格我最強~
02/21 18:53, 222F

02/21 20:23, , 223F
XDDDD某空格魔人太好笑了 一堆更重要的要看在那邊挑空格
02/21 20:23, 223F

02/23 01:21, , 224F
請問有人遇過規定要空3格的嗎? 而不是TAB的空四格
02/23 01:21, 224F

02/23 01:21, , 225F
有點受不了這規定...
02/23 01:21, 225F

02/23 04:53, , 226F
tab space還有6和8的 如果是插入tab倒還好 space就只能習慣
02/23 04:53, 226F

02/23 11:20, , 227F
@prpure 可用工具將tab改為3格你會好受點
02/23 11:20, 227F

02/23 21:51, , 228F
寫完再用tool一次改,astyle可以設
02/23 21:51, 228F

02/23 21:51, , 229F
tab等於幾個空白或是真的就是tab
02/23 21:51, 229F

02/23 21:52, , 230F
我寫的時候都只按tab再靠astyle幫忙format
02/23 21:52, 230F

02/23 21:54, , 231F
寫linux kernel就是8個,寫自己的程式就是4個,很方便
02/23 21:54, 231F

02/23 21:55, , 232F
也不用背,規則寫好一次轉完
02/23 21:55, 232F

02/23 21:56, , 233F
只是怕大家都用tool,某些空格工程師可能會失業~
02/23 21:56, 233F

02/25 01:04, , 234F
ctrl+alt+f 結束
02/25 01:04, 234F
文章代碼(AID): #1Kv5k5wo (Soft_Job)
討論串 (同標題文章)
本文引述了以下文章的的內容:
以下文章回應了本文 (最舊先):
完整討論串 (本文為第 7 之 12 篇):
文章代碼(AID): #1Kv5k5wo (Soft_Job)