git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v10 12/12] bisect--helper: `get_terms` & `bisect_terms` shell function in C

From
Pranit Bauva <pranit.bauva@gmail.com>
Date
Jul 27, 2016, 04:20 UTC
Message-ID
<CAFZEwPP=AL6V+AitmVD6qnt7FvUJiX1wqrn5C2UjEKxAnnumew@mail.gmail.com>
In-Reply-To
<xmqqmvl41f0x.fsf@gitster.mtv.corp.google.com>
Hey Junio,
On Tue, Jul 26, 2016 at 11:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 23 quoted lines
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> On 07/25/2016 06:53 PM, Junio C Hamano wrote:
>>> Pranit Bauva <pranit.bauva@gmail.com> writes:
>>>
>>>>>> >>> +enum terms_defined {
>>>>>> >>> +       TERM_BAD = 1,
>>>>>> >>> +       TERM_GOOD = 2,
>>>>>> >>> +       TERM_NEW = 4,
>>>>>> >>> +       TERM_OLD = 8
>>>>>> >>> +};
>>>>>> >>> +
>>>>> >> ...
>> Is there any risk that a more generic term like "TERM_BAD" may collide
>> with some other definition some day ?
>>
>> Would it make sense to call it GIT_BISECT_TERM_BAD, GBS_TERM_BAD,
>> BIS_TERM_BAD or something more unique ?
>
> I am not sure if the scope of these symbols would ever escape
> outside bisect-helper.c (and builtin/bisect.c eventually when we
> retire git-bisect.sh), but BISECT_TERM_{GOOD,BAD,OLD,NEW} would not
> be too bad.

I agree that it wouldn't be too bad. This can be considered as low hanging fruit and picked up after the completion of the project as after the whole conversion, some re-ordering of code would need to be done. For eg. there is read_bisect_terms() is in bisect.c while get_terms() is in builtin/bisect--helper.c but they both do the same stuff except the later one uses strbuf and a lot more important stuff. Maybe after the whole conversion, the above enum (or #define bitmap) should also be moved to bisect.h and be used consistently in bisect.c too.

Regards, Pranit Bauva

Previous: Junio C HamanoNext: Junio C Hamano
Message 10 of 20 in “bisect--helper: use OPT_CMDMODE instead of OPT_BOOL”
  1. 01/12 bisect--helper: use OPT_CMDMODE instead of OPT_BOOLPranit Bauva, Jul 20, 2016
  2. 06/12 wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()Pranit Bauva, Jul 20, 2016
  3. 12/12 bisect--helper: `get_terms` & `bisect_terms` shell function in CPranit Bauva, Jul 20, 2016
  4. Torsten BögershausenJul 22, 2016
  5. Pranit BauvaJul 22, 2016
  6. Junio C HamanoJul 25, 2016
  7. Christian CouderJul 25, 2016
  8. Torsten BögershausenJul 26, 2016
  9. Junio C HamanoJul 26, 2016
  10. Pranit BauvaJul 27, 2016
  11. Junio C HamanoJul 27, 2016
  12. 10/12 bisect--helper: `check_and_set_terms` shell function in CPranit Bauva, Jul 20, 2016
  13. 11/12 bisect--helper: `bisect_next_check` shell function in CPranit Bauva, Jul 20, 2016
  14. 04/12 bisect--helper: `bisect_clean_state` shell function in CPranit Bauva, Jul 20, 2016
  15. 05/12 t6030: explicitly test for bisection cleanupPranit Bauva, Jul 20, 2016
  16. 02/12 bisect: rewrite `check_term_format` shell function in CPranit Bauva, Jul 20, 2016
  17. 03/12 bisect--helper: `write_terms` shell function in CPranit Bauva, Jul 20, 2016
  18. 07/12 bisect--helper: `bisect_reset` shell function in CPranit Bauva, Jul 20, 2016
  19. 09/12 bisect--helper: `bisect_write` shell function in CPranit Bauva, Jul 20, 2016
  20. 08/12 bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in CPranit Bauva, Jul 20, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.