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
next prev 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.