From: Christian Couder <christian.couder@gmail.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Miriam Rubio <mirucam@gmail.com>, git <git@vger.kernel.org>,
Pranit Bauva <pranit.bauva@gmail.com>,
Christian Couder <chriscool@tuxfamily.org>,
Tanushree Tumane <tanushreetumane@gmail.com>
Subject: Re: [PATCH 11/29] bisect: libify `bisect_next_all`
Date: Tue, 21 Jan 2020 08:15:59 +0100 [thread overview]
Message-ID: <CAP8UFD3QP3QXe32cdK+q=P+v3dK1Bfws5KgNnm-JfDnPkpBZfw@mail.gmail.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.2001202325000.46@tvgsbejvaqbjf.bet>
On Mon, Jan 20, 2020 at 11:29 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Mon, 20 Jan 2020, Miriam Rubio wrote:
> > Since we want to get rid of git-bisect.sh it would be necessary to
> > convert those exit() calls to return statements so that errors can be
> > reported.
> >
> > Emulate try catch in C by converting `exit(<positive-value>)` to
> > `return <negative-value>`. Follow POSIX conventions to return
> > <negative-value> to indicate error.
> >
> > Turn `exit()` to `return` calls in `bisect_next_all()`.
> >
> > All the functions calling `bisect_next_all()` are already able to
> > handle return values from it.
>
> So this patch brings more magic values that really would like to become
> `enum` values. At this point, we're talking about `bisect_next_all()`
> which is _not_ a file-local (`static`) function. Therefore, we now really
> wade into API territory where an `enum` is no longer just a nice thing,
> but a necessary thing: `bisect_next_all()` is a function that can be
> called from other source files.
I agree that return values could be better documented. About enum
though, I am perhaps wrong but I don't think that we use them often
for error codes. Do you have examples in the code base where they are
used for such a purpose along with regular `error(...)` calls?
> As far as I can see, this patch concludes the "libify" part of the patch
> series (read: it would have been the second patch series I would have
> suggested to split out from an otherwise too-long-to-be-reviewed set of
> patches, if I had been your mentor),
I am ok with saying that "part 1" stops after this patch and that the
rest will be "part 2" and will not be resent in the version 2 of the
"part 1" patch series, but will rather be resent later, when "part 1"
will be in next for example.
> and I'll leave it at that for
> tonight; Hopefully I will find some time to review more of these patches
> tomorrow.
That would be great!
Thanks,
Christian.
next prev parent reply other threads:[~2020-01-21 7:16 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-20 14:37 [Outreachy][PATCH 00/29] Finish converting git bisect to C part 1 Miriam Rubio
2020-01-20 14:37 ` [PATCH 01/29] bisect--helper: convert `vocab_*` char pointers to char arrays Miriam Rubio
2020-01-20 14:37 ` [PATCH 02/29] bisect--helper: change `retval` to `res` Miriam Rubio
2020-01-20 14:37 ` [PATCH 03/29] bisect: use the standard 'if (!var)' way to check for 0 Miriam Rubio
2020-01-20 14:37 ` [PATCH 04/29] run-command: make `exists_in_PATH()` non-static Miriam Rubio
2020-01-20 14:37 ` [PATCH 05/29] bisect--helper: introduce new `decide_next()` function Miriam Rubio
2020-01-20 14:37 ` [PATCH 06/29] bisect: libify `exit_if_skipped_commits` to `error_if_skipped*` and its dependents Miriam Rubio
2020-01-20 21:57 ` Johannes Schindelin
2020-01-21 6:40 ` Christian Couder
2020-01-21 10:00 ` Miriam R.
2020-01-20 14:37 ` [PATCH 07/29] bisect: libify `bisect_checkout` Miriam Rubio
2020-01-20 14:37 ` [PATCH 08/29] bisect: libify `check_merge_bases` and its dependents Miriam Rubio
2020-01-20 22:09 ` Johannes Schindelin
2020-01-21 9:59 ` Miriam R.
2020-01-20 14:37 ` [PATCH 09/29] bisect: libify `check_good_are_ancestors_of_bad` " Miriam Rubio
2020-01-20 22:20 ` Johannes Schindelin
2020-01-21 6:59 ` Christian Couder
2020-01-21 10:00 ` Miriam R.
2020-01-20 14:37 ` [PATCH 10/29] bisect: libify `handle_bad_merge_base` " Miriam Rubio
2020-01-20 22:23 ` Johannes Schindelin
2020-01-21 7:05 ` Christian Couder
2020-01-21 10:00 ` Miriam R.
2020-01-20 14:37 ` [PATCH 11/29] bisect: libify `bisect_next_all` Miriam Rubio
2020-01-20 22:29 ` Johannes Schindelin
2020-01-21 7:15 ` Christian Couder [this message]
2020-01-30 15:18 ` Johannes Schindelin
2020-01-20 14:37 ` [PATCH 12/29] bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C Miriam Rubio
2020-01-30 22:47 ` Johannes Schindelin
2020-01-31 10:53 ` Miriam R.
2020-02-17 7:20 ` Christian Couder
2020-02-17 22:00 ` Johannes Schindelin
2020-01-20 14:37 ` [PATCH 13/29] bisect--helper: finish porting `bisect_start()` to C Miriam Rubio
2020-01-20 14:37 ` [PATCH 14/29] bisect--helper: retire `--bisect-clean-state` subcommand Miriam Rubio
2020-01-20 14:37 ` [PATCH 15/29] bisect--helper: retire `--next-all` subcommand Miriam Rubio
2020-01-20 14:37 ` [PATCH 16/29] bisect--helper: reimplement `bisect_autostart` shell function in C Miriam Rubio
2020-01-20 14:37 ` [PATCH 17/29] bisect--helper: reimplement `bisect_state` & `bisect_head` shell functions " Miriam Rubio
2020-01-20 14:37 ` [PATCH 18/29] bisect--helper: retire `--check-expected-revs` subcommand Miriam Rubio
2020-01-20 14:37 ` [PATCH 19/29] bisect--helper: retire `--write-terms` subcommand Miriam Rubio
2020-01-20 14:37 ` [PATCH 20/29] bisect--helper: reimplement `bisect_log` shell function in C Miriam Rubio
2020-01-20 14:37 ` [PATCH 21/29] bisect--helper: reimplement `bisect_replay` " Miriam Rubio
2020-01-20 14:37 ` [PATCH 22/29] bisect--helper: retire `--bisect-write` subcommand Miriam Rubio
2020-01-20 14:37 ` [PATCH 23/29] bisect--helper: use `res` instead of return in BISECT_RESET case option Miriam Rubio
2020-01-20 14:37 ` [PATCH 24/29] bisect--helper: retire `--bisect-autostart` subcommand Miriam Rubio
2020-01-20 14:37 ` [PATCH 25/29] bisect--helper: retire `--bisect-auto-next` subcommand Miriam Rubio
2020-01-20 14:37 ` [PATCH 26/29] bisect--helper: reimplement `bisect_skip` shell function in C Miriam Rubio
2020-01-20 14:37 ` [PATCH 27/29] bisect--helper: retire `--check-and-set-terms` subcommand Miriam Rubio
2020-01-20 14:37 ` [PATCH 28/29] bisect--helper: reimplement `bisect_visualize()`shell function in C Miriam Rubio
2020-01-20 14:38 ` [PATCH 29/29] bisect--helper: reimplement `bisect_run` shell " Miriam Rubio
2020-01-20 21:41 ` [Outreachy][PATCH 00/29] Finish converting git bisect to C part 1 Johannes Schindelin
2020-01-20 23:24 ` Christian Couder
2020-01-30 15:12 ` Johannes Schindelin
2020-01-30 21:12 ` Junio C Hamano
2020-01-21 8:44 ` Miriam R.
2020-01-30 15:13 ` 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAP8UFD3QP3QXe32cdK+q=P+v3dK1Bfws5KgNnm-JfDnPkpBZfw@mail.gmail.com' \
--to=christian.couder@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=chriscool@tuxfamily.org \
--cc=git@vger.kernel.org \
--cc=mirucam@gmail.com \
--cc=pranit.bauva@gmail.com \
--cc=tanushreetumane@gmail.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.