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

Re: [PATCH v5 0/2] bisect--helper: rewrite of check-term-format()

From
Pranit Bauva <pranit.bauva@gmail.com>
Date
May 8, 2016, 06:23 UTC
Message-ID
<CAFZEwPML8MCAL2quAN3ExMFXrhjS07g3KEC-WuSfYAXc2WQKuQ@mail.gmail.com>
In-Reply-To
<xmqq60up1e24.fsf@gitster.mtv.corp.google.com>
On Sun, May 8, 2016 at 7:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 13 quoted lines
> Pranit Bauva <pranit.bauva@gmail.com> writes:
>
>> I completely missed your point and you want me to go the Eric Sunshine's way?
>
> I am neutral.
>
> When I read your response to Eric's "top down" suggestion, I didn't
> quite get much more than "I started going this way, and I do not
> want to change to the other direction.", which was what I had the
> most trouble with.  If your justification for the approach to start
> from building a tiny bottom layer that will need to be dismantled
> soon and repeat that (which sounds somewhat wasteful) were more
> convincing, I may have felt differently.

Sorry if it seemed that "I have done quite some work and I don't want to scrape it off and redo everything". This isn't a case for me. I think of this as just a small part in the process of learning and my efforts would be completely wasted as I can still reuse the methods I wrote. This is still open for a "philosophical" discussion. I am assuming 1e1ea69fa4e is how Eric is suggesting.

Why I think its better to go the subcommand way:
 * I can test **C implementation** of the function to verify whether
it was done in a proper way. By using a "top down" approach, I can
make the test suite passing by running the code from git-bisect.sh not
the re-written C code.
 * I have made a very few minor mistakes in conversion of
check_term_format() which just messed up my head (I actually spent 3
days before Christian pointed out that '!' was missing). Imagine
debugging the complete git-bisect.sh to find a very small error after
a complete implementation.
 * Using subcommands, I can easily verify whether I have done it the
right way or not.
 * It makes the review process very simple. The reviewers/testers can
verify that my method is working correctly and well computers can
detect errors better than humans.
 * I can convert small functions which can be reviewed easily rather
than dumping a big series.

What I think is that the bottom up approach will make life easier for the me and for reviewer. Yes, it does make the code "unclean" for the time being and if it is between releases then all the more painful. Well, the latter part can be avoided by keeping it in next.

Please point out if I am mistaken about anything.

Regards, Pranit Bauva

Previous: Junio C HamanoNext: Pranit Bauva
Message 69 of 110 in “bisect--helper: convert a function in shell to C”
  1. bisect--helper: convert a function in shell to CPranit Bauva, Mar 21, 2016
  2. Stefan BellerMar 22, 2016
  3. Christian CouderMar 22, 2016
  4. Pranit BauvaMar 22, 2016
  5. Pranit BauvaMar 22, 2016
  6. bisect--helper: convert a function in shell to CPranit Bauva, Mar 22, 2016
  7. Johannes SchindelinMar 22, 2016
  8. Johannes SchindelinMar 22, 2016
  9. Pranit BauvaMar 22, 2016
  10. Johannes SchindelinMar 23, 2016
  11. Junio C HamanoMar 22, 2016
  12. Johannes SchindelinMar 22, 2016
  13. Pranit BauvaMar 22, 2016
  14. Stefan BellerMar 22, 2016
  15. Johannes SchindelinMar 23, 2016
  16. Pranit BauvaMar 22, 2016
  17. Johannes SchindelinMar 23, 2016
  18. Pranit BauvaMar 23, 2016
  19. bisect--helper: convert a function in shell to CPranit Bauva, Mar 23, 2016
  20. Johannes SchindelinMar 23, 2016
  21. Pranit BauvaMar 23, 2016
  22. Junio C HamanoMar 23, 2016
  23. Pranit BauvaMar 23, 2016
  24. Junio C HamanoMar 23, 2016
  25. Pranit BauvaMar 23, 2016
  26. 0/2 bisect--helper: rewrite of check_term_format()Pranit Bauva, May 4, 2016
  27. 1/2 bisect--helper: use OPT_CMDMODE instead of OPT_BOOLPranit Bauva, May 4, 2016
  28. Pranit BauvaMay 4, 2016
  29. Eric SunshineMay 4, 2016
  30. Christian CouderMay 4, 2016
  31. Johannes SchindelinMay 4, 2016
  32. Pranit BauvaMay 4, 2016
  33. Christian CouderMay 4, 2016
  34. Johannes SchindelinMay 4, 2016
  35. Christian CouderMay 4, 2016
  36. Johannes SchindelinMay 4, 2016
  37. Christian CouderMay 4, 2016
  38. Johannes SchindelinMay 8, 2016
  39. Junio C HamanoMay 4, 2016
  40. Johannes SchindelinMay 4, 2016
  41. 2/2 bisect: rewrite `check_term_format` shell function in CPranit Bauva, May 4, 2016
  42. Eric SunshineMay 4, 2016
  43. Pranit BauvaMay 4, 2016
  44. Pranit BauvaMay 4, 2016
  45. Eric SunshineMay 4, 2016
  46. Christian CouderMay 4, 2016
  47. Pranit BauvaMay 4, 2016
  48. Eric SunshineMay 4, 2016
  49. Pranit BauvaMay 4, 2016
  50. Johannes SchindelinMay 4, 2016
  51. Pranit BauvaMay 4, 2016
  52. Christian CouderMay 4, 2016
  53. Junio C HamanoMay 4, 2016
  54. Christian CouderMay 4, 2016
  55. Pranit BauvaMay 4, 2016
  56. 0/2 bisect--helper: rewrite of check-term-format()Pranit Bauva, May 6, 2016
  57. 1/2 bisect--helper: use OPT_CMDMODE instead of OPT_BOOLPranit Bauva, May 6, 2016
  58. Johannes SchindelinMay 8, 2016
  59. Pranit BauvaMay 8, 2016
  60. Christian CouderMay 8, 2016
  61. Pranit BauvaMay 8, 2016
  62. Johannes SchindelinMay 9, 2016
  63. Pranit BauvaMay 9, 2016
  64. 2/2 bisect: rewrite `check_term_format` shell function in CPranit Bauva, May 6, 2016
  65. Eric SunshineMay 7, 2016
  66. Junio C HamanoMay 6, 2016
  67. Pranit BauvaMay 7, 2016
  68. Junio C HamanoMay 8, 2016
  69. Pranit BauvaMay 8, 2016
  70. Pranit BauvaMay 8, 2016
  71. Eric SunshineMay 16, 2016
  72. Christian CouderMay 16, 2016
  73. Pranit BauvaMay 20, 2016
  74. 0/3 bisect--helper: check_term_format() & write_terms()Pranit Bauva, May 12, 2016
  75. 1/3 bisect--helper: use OPT_CMDMODE instead of OPT_BOOLPranit Bauva, May 12, 2016
  76. Eric SunshineMay 16, 2016
  77. Pranit BauvaMay 20, 2016
  78. 2/3 bisect: rewrite `check_term_format` shell function in CPranit Bauva, May 12, 2016
  79. Junio C HamanoMay 12, 2016
  80. Pranit BauvaMay 13, 2016
  81. Junio C HamanoMay 13, 2016
  82. 3/3 bisect--helper: `write_terms` shell function in CPranit Bauva, May 12, 2016
  83. Eric SunshineMay 16, 2016
  84. Johannes SchindelinMay 16, 2016
  85. Eric SunshineMay 16, 2016
  86. Johannes SchindelinMay 16, 2016
  87. Eric SunshineMay 16, 2016
  88. Pranit BauvaMay 20, 2016
  89. Johannes SchindelinMay 23, 2016
  90. Christian CouderMay 23, 2016
  91. Johannes SchindelinMay 23, 2016
  92. Pranit BauvaMay 23, 2016
  93. Pranit BauvaMay 20, 2016
  94. 0/3 bisect--helper: check_term_format() & write_terms()Pranit Bauva, May 23, 2016
  95. 1/3 bisect--helper: use OPT_CMDMODE instead of OPT_BOOLPranit Bauva, May 23, 2016
  96. 2/3 bisect: rewrite `check_term_format` shell function in CPranit Bauva, May 23, 2016
  97. 3/3 bisect--helper: `write_terms` shell function in CPranit Bauva, May 23, 2016
  98. Eric SunshineMay 23, 2016
  99. Pranit BauvaMay 23, 2016
  100. 0/3 bisect--helper: check-term-format() & write_terms()Pranit Bauva, May 24, 2016
  101. 1/3 bisect--helper: use OPT_CMDMODE instead of OPT_BOOLPranit Bauva, May 24, 2016
  102. 2/3 bisect: rewrite `check_term_format` shell function in CPranit Bauva, May 24, 2016
  103. Johannes SchindelinMay 25, 2016
  104. Pranit BauvaMay 25, 2016
  105. Lars SchneiderJun 16, 2016
  106. Pranit BauvaJun 17, 2016
  107. 3/3 bisect--helper: `write_terms` shell function in CPranit Bauva, May 24, 2016
  108. Christian CouderMay 24, 2016
  109. Junio C HamanoMay 24, 2016
  110. Pranit BauvaMay 24, 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.