From: Junio C Hamano <firstname.lastname@example.org> To: Eric Sunshine <email@example.com> Cc: "Ævar Arnfjörð Bjarmason" <firstname.lastname@example.org>, "Pratik Karki" <email@example.com>, "Git List" <firstname.lastname@example.org> Subject: Re: [GSoC] [PATCH] test: avoid pipes in git related commands for test suite Date: Thu, 15 Mar 2018 10:04:01 -0700 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <CAPig+cTLCswg_=q5ybnyN3As4Au05q5eAcA7Prr643KCgZ0OAw@mail.gmail.com> (Eric Sunshine's message of "Wed, 14 Mar 2018 14:22:45 -0400") Eric Sunshine <firstname.lastname@example.org> writes: > Thanks for presenting an opposing opinion. While I understand your > position, the reason for my suggested transformation is that if the > patch already transformed the code in the way suggested, it would > increase my confidence, as a reviewer, that the patch author had > _studied_ and _understood_ the code. Increased confidence is > especially important for mechanical transformations since -- as seen > in the unsnipped review comment below -- blindly-applied mechanical > transformations can be suboptimal or outright incorrect. > > It's also the sort of review comment I would make even to very > seasoned project participants. > > : https://public-inbox.org/git/CAPig+cQLmYQeRhPxvZHmY7gApnbE25H_KoSWs-ZjuBo4BruimQ@mail.gmail.com/ Yes, it is a good example that mechanical conversions are often mind-numbing and make even seasoned participants miss trivially obvious improvement opportunities ;-) It however is OK to be more lenient to newer participants and allow deferring such "while at it, make it right" on top of "minimally required for correctness", in order to encourage them by getting something to the tree early ;-)
next prev parent reply other threads:[~2018-03-15 17:04 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-03-13 20:19 Pratik Karki 2018-03-14 7:30 ` Eric Sunshine 2018-03-14 9:57 ` Ævar Arnfjörð Bjarmason 2018-03-14 18:22 ` Eric Sunshine 2018-03-15 17:04 ` Junio C Hamano [this message] 2018-03-19 17:32 ` [GSoC][PATCH] " Pratik Karki 2018-03-21 11:02 ` Eric Sunshine 2018-03-21 15:23 ` [GSoC][PATCH v3] test: avoid pipes in git related commands for test Pratik Karki 2018-03-21 18:11 ` Junio C Hamano 2018-03-21 18:45 ` Eric Sunshine 2018-03-21 18:58 ` Eric Sunshine 2018-03-23 15:01 ` [GSoC][PATCH v4] " Pratik Karki 2018-03-25 8:37 ` Eric Sunshine 2018-03-27 17:31 ` [GSoC][PATCH v5] " Pratik Karki 2018-03-30 21:45 ` Eric Sunshine 2018-03-30 22:08 ` Junio C Hamano
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [GSoC] [PATCH] test: avoid pipes in git related commands for test suite' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).