From: Elijah Newren <firstname.lastname@example.org> To: Junio C Hamano <email@example.com> Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>, Elijah Newren via GitGitGadget <firstname.lastname@example.org>, Git Mailing List <email@example.com>, Philip Oakley <firstname.lastname@example.org>, Phillip Wood <email@example.com> Subject: Re: [PATCH v2] sequencer: fix edit handling for cherry-pick and revert messages Date: Thu, 1 Apr 2021 09:31:26 -0700 [thread overview] Message-ID: <CABPp-BFLyYTboiGkP=sc7S+stRZvozAqnJPnza+M0q2nakvD2Q@mail.gmail.com> (raw) In-Reply-To: <firstname.lastname@example.org> On Wed, Mar 31, 2021 at 11:01 AM Junio C Hamano <email@example.com> wrote: > > Elijah Newren <firstname.lastname@example.org> writes: > > > ... would you two prefer > > that I add a new affirm() function that is ALSO compiled out in > > production? > > The reason I do not think affirm would fly is because it shares the > most problematic trait with assert(): it can be comipled out. > > There was at least one bug we found and fixed in a piece of code > that came later in a codepath that had an assert() in it. The buggy > code (IIRC, it was added way after the assert() was written in an > unrelated patch) was unknowingly depending on a side-effect of an > innocuous function call (i.e. assert(func_tion(args) == expected)) > made in the assert condition, and the bug did not reproduce in the > developer's environment. Ah, now that's a much different argument than claiming assert is useless. I had to debug one of those once about a decade ago, in a different project, put in by some other developer, and yeah it's painful. If that causes assert() to be frowned upon, then I can understand based on this reason. If so, perhaps a BUG_ON() function would be useful? The "if (...) BUG()" construct causes line coverage problems and I find it an onerous way to signal to future code readers "hey, just as a reminder, these things should be true at this point". If you need to signal something more than that or explain why the things being false is a problem or what might cause it, then the "if (...) BUG()" construct is useful and there are certainly many places for that, but it just doesn't fit all the cases.
next prev parent reply other threads:[~2021-04-01 17:51 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-26 7:16 [PATCH] " Elijah Newren via GitGitGadget 2021-03-26 12:27 ` Philip Oakley 2021-03-26 15:12 ` Elijah Newren 2021-03-28 1:30 ` Junio C Hamano 2021-03-29 9:23 ` Phillip Wood 2021-03-29 20:52 ` Junio C Hamano 2021-03-29 21:25 ` Elijah Newren 2021-03-30 2:09 ` [PATCH v2] " Elijah Newren via GitGitGadget 2021-03-30 10:13 ` Johannes Schindelin 2021-03-30 18:47 ` Junio C Hamano 2021-03-30 20:16 ` Elijah Newren 2021-03-31 17:36 ` Ævar Arnfjörð Bjarmason 2021-03-31 17:52 ` Elijah Newren 2021-03-31 18:01 ` Junio C Hamano 2021-04-01 16:31 ` Elijah Newren [this message] 2021-03-30 19:37 ` Elijah Newren 2021-03-31 13:48 ` unifying sequencer's options persisting, was " Johannes Schindelin 2021-04-02 11:28 ` Phillip Wood 2021-04-02 13:10 ` Phillip Wood 2021-04-02 21:01 ` Junio C Hamano 2021-04-02 22:18 ` Elijah Newren 2021-04-02 22:27 ` Junio C Hamano 2021-04-08 2:40 ` Johannes Schindelin 2021-04-08 17:45 ` Junio C Hamano 2021-04-08 19:58 ` Christian Couder 2021-04-09 13:53 ` Johannes Schindelin 2021-03-31 6:52 ` [PATCH v3] " Elijah Newren via GitGitGadget 2021-03-31 14:38 ` Johannes Schindelin
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 \ --in-reply-to='CABPp-BFLyYTboiGkP=sc7S+stRZvozAqnJPnza+M0q2nakvD2Q@mail.gmail.com' \ --email@example.com \ --cc=Johannes.Schindelin@gmx.de \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH v2] sequencer: fix edit handling for cherry-pick and revert messages' \ /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).