All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jacob Stopak <jacob@initialcommit.io>
To: git@vger.kernel.org
Cc: Jacob Stopak <jacob@initialcommit.io>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v3 0/1] bugreport: include +i in outfile suffix as needed
Date: Mon, 16 Oct 2023 14:40:44 -0700	[thread overview]
Message-ID: <20231016214045.146862-1-jacob@initialcommit.io> (raw)
In-Reply-To: <20231015034238.100675-2-jacob@initialcommit.io>

Similar functionality to the previous patch version but with the
following changes:

  * Remove the default_option_suffix variable and clean up the way the
    option_suffix value is set.

  * Refactor code for building report path and diagnostics zip file path
    into the new function build_path(...), which builds a fresh path
    from scratch each time it's called. Although it does take quite a few
    arguments, it reduces duplicated code and simplifies the code by
    removing the need for splicing or inserting the incrementable suffix.

  * Allow the increment value 'i' to grow as needed instead of stopping
    at 9.

  * Replace xopen() with open() so that the program doesn't die with an
    error if the file already exists.

  * Replace the while loop with a fallback loop to prevent TOCTOU.

  * Clean up commit message verbiage.

Some additional comments:

> Perhaps we should refine the error message we give in this case and
> we are done, then?

I had thought about this but due to the variable timed behavior I had
trouble coming up with a message that would convey clearly what's going
on and what the user should do. Here were some things that popped into
my head but they all sounded a bit silly to me:

"A bugreport with this name already exists, try again shortly"
or
"File exists: wait 1 minute and try again or use a custom suffix with -s"
or
"File exists: try again in 1 to 60 seconds :-P"

I think the reason they sound silly to me is that the user is only being
made aware of this info because the program happens to be operating this
way - instead of the program working in a smoother way where this type of
operational info wouldn't need to be conveyed.

> What downside do you see in using 2023-10-15-1008+10 after you tried
> 9 of them?  The code to limit the upper bound smells like a wasted
> effort that helps nobody in practice because it is "unlikely".

> And limiting the upper bound also means you now have to have extra
> code to deal with "we ran out---error out and help the user how to
> recover" anyway.

I completely agree with this and made these updates.

> Notice that you said "in a single minute" without "calendar" and the
> sentence is perfectly understandable?

I googled "calendar minute" and the only thing that came up was some
Java documentation... So I guess I just made it up... :'D

Jacob Stopak (1):
  bugreport: include +i in outfile suffix as needed

 builtin/bugreport.c | 83 +++++++++++++++++++++++++++++++--------------
 1 file changed, 57 insertions(+), 26 deletions(-)


base-commit: 493f4622739e9b64f24b465b21aa85870dd9dc09
-- 
2.42.0.297.g36452639b8


  parent reply	other threads:[~2023-10-16 21:41 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-14  4:01 [PATCH] bugreport: add 'seconds' to default outfile name Jacob Stopak
2023-10-14 10:35 ` Kristoffer Haugsbakk
2023-10-14 16:27 ` Junio C Hamano
2023-10-14 16:33   ` Dragan Simic
2023-10-14 17:45     ` Junio C Hamano
2023-10-14 17:52       ` Dragan Simic
2023-10-15  3:07         ` Jacob Stopak
2023-10-15  3:13           ` Dragan Simic
2023-10-15  3:01   ` Jacob Stopak
2023-10-15 17:06     ` Junio C Hamano
2023-10-15  3:42 ` [PATCH v2 0/3] bugreport: include +i in outfile suffix as needed Jacob Stopak
2023-10-15  3:42   ` [PATCH v2 1/3] " Jacob Stopak
2023-10-15 17:36     ` Junio C Hamano
2023-10-16 21:40     ` Jacob Stopak [this message]
2023-10-16 21:40       ` [PATCH v3 1/1] " Jacob Stopak
2023-10-16 22:55         ` Junio C Hamano
2023-10-17  3:17           ` Jacob Stopak
2023-10-21  0:39         ` Junio C Hamano
2023-10-26 21:19         ` Emily Shaffer
2023-10-27  6:34           ` Jacob Stopak
2024-01-06  4:54             ` Jacob Stopak
2023-10-15  3:42   ` [PATCH v2 2/3] bugreport: match diagnostics filename with report Jacob Stopak
2023-10-15  3:42   ` [PATCH v2 3/3] bugreport: don't create --diagnose zip w/o report Jacob Stopak

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231016214045.146862-1-jacob@initialcommit.io \
    --to=jacob@initialcommit.io \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.