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

Re: [WIP/PATCH v4 8/8] ref-filter: add 'ref-filter.h'

From
Eric Sunshine <sunshine@sunshineco.com>
Date
May 31, 2015, 08:29 UTC
Message-ID
<CAPig+cRF7VEqqBWJY_GRVL+8YCfrD-mYJ2idN1yzVE7JKeTpwA@mail.gmail.com>
In-Reply-To
<556AC40C.5020205@gmail.com>
On Sun, May 31, 2015 at 4:19 AM, Karthik Nayak <karthik.188@gmail.com> wrote:
Show 23 quoted lines
> On 05/31/2015 09:13 AM, Eric Sunshine wrote:
>> On Sat, May 30, 2015 at 1:53 PM, Karthik Nayak <karthik.188@gmail.com>
>> wrote:
>>>
>>> Create 'ref-filter.h', also add ref-filter to the Makefile.
>>> This completes movement of creation of 'ref-filter' from
>>> 'for-each-ref'.
>>
>> It's important that the project can be built successfully and function
>> correctly at each stage of a patch series. Unfortunately, the way this
>> series is organized, the build breaks after application of patch 7/8
>> since for-each-ref.c is trying to access functionality which was moved
>> to ref-filter.c, but there is no header at that stage which advertises
>> the functionality. Fixing this may be as simple as swapping patches
>> 7/8 and 8/8, along with whatever minor adjustments they might need to
>> keep them sane. (The alternative would be to combine patches 7/8 and
>> 8/8, however, Matthieu didn't seem to favor that approach[1].)
>
> That is kind of a problem, If I need to swap those commits also, I'd have to
> add the part where ref-filter is added to the Makefile with the code
> movement from for-each-ref to ref-filter. This again will not just be Code
> movement.
> But I guess thats a fair trade-off, will change it. Thanks

Don't take the "code movement-only" recommendation too literally. What people mean is that you shouldn't make changes to the lines you're moving from one location to another (other than absolutely mandatory changes to support the movement) since it's difficult to spot and review changes in lines which are being moved.

The recommendation is not saying that the patch itself can't include any other changes (though, it's often best to minimize other changes in the same patch). In this case, the Makefile change is logically related to that particular code movement, so it's correct to include it in the patch. (Also, the Makefile change is so minor that it's not a burden upon reviewers.)

Previous: Karthik NayakNext: Karthik Nayak
Message 37 of 44 in “for-each-ref: extract helper functions out of grab_single_ref()”
  1. 1/8 for-each-ref: extract helper functions out of grab_single_ref()Karthik Nayak, May 30, 2015
  2. Eric SunshineMay 31, 2015
  3. Karthik NayakMay 31, 2015
  4. Junio C HamanoMay 31, 2015
  5. Karthik NayakMay 31, 2015
  6. Matthieu MoyMay 31, 2015
  7. Junio C HamanoJun 1, 2015
  8. 2/8 for-each-ref: simplify codeKarthik Nayak, May 30, 2015
  9. 3/8 for-each-ref: rename 'refinfo' to 'ref_array_item'Karthik Nayak, May 30, 2015
  10. Junio C HamanoMay 31, 2015
  11. Karthik NayakMay 31, 2015
  12. 4/8 for-each-ref: introduce new structures for better organisationKarthik Nayak, May 30, 2015
  13. Eric SunshineMay 31, 2015
  14. Karthik NayakMay 31, 2015
  15. 5/8 for-each-ref: introduce 'ref_filter_clear_data()'Karthik Nayak, May 30, 2015
  16. Christian CouderMay 31, 2015
  17. Karthik NayakMay 31, 2015
  18. 6/8 for-each-ref: rename some functions and make them publicKarthik Nayak, May 30, 2015
  19. Eric SunshineMay 31, 2015
  20. Karthik NayakMay 31, 2015
  21. Christian CouderMay 31, 2015
  22. Christian CouderMay 31, 2015
  23. Karthik NayakMay 31, 2015
  24. Christian CouderMay 31, 2015
  25. Karthik NayakMay 31, 2015
  26. Matthieu MoyJun 1, 2015
  27. Karthik NayakJun 1, 2015
  28. Junio C HamanoMay 31, 2015
  29. Junio C HamanoMay 31, 2015
  30. Karthik NayakMay 31, 2015
  31. Junio C HamanoJun 1, 2015
  32. Karthik NayakJun 1, 2015
  33. 7/8 ref-filter: move code from 'for-each-ref'Karthik Nayak, May 30, 2015
  34. 8/8 ref-filter: add 'ref-filter.h'Karthik Nayak, May 30, 2015
  35. Eric SunshineMay 31, 2015
  36. Karthik NayakMay 31, 2015
  37. Eric SunshineMay 31, 2015
  38. Karthik NayakMay 31, 2015
  39. Matthieu MoyMay 31, 2015
  40. Karthik NayakMay 31, 2015
  41. Christian CouderMay 31, 2015
  42. Matthieu MoyJun 1, 2015
  43. Christian CouderMay 31, 2015
  44. Karthik NayakMay 31, 2015

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.