Re: [PATCH v8 00/41] libify apply and use lib in am, part 2

2 messages, 2 authors, 2016-07-27 · open the first message on its own page

Re: [PATCH v8 00/41] libify apply and use lib in am, part 2

From: Junio C Hamano <hidden>
Date: 2016-07-26 21:18:21

Christian Couder [off-list ref] writes:
Sorry if this patch series is still long. I can split it into two or
more series if it is prefered.
...
Christian Couder (41):
  apply: make some names more specific
  apply: move 'struct apply_state' to apply.h
  builtin/apply: make apply_patch() return -1 or -128 instead of
    die()ing
  builtin/apply: read_patch_file() return -1 instead of die()ing
  builtin/apply: make find_header() return -128 instead of die()ing
  builtin/apply: make parse_chunk() return a negative integer on error
  builtin/apply: make parse_single_patch() return -1 on error
  builtin/apply: make parse_whitespace_option() return -1 instead of
    die()ing
  builtin/apply: make parse_ignorewhitespace_option() return -1 instead
    of die()ing
  builtin/apply: move init_apply_state() to apply.c
  apply: make init_apply_state() return -1 instead of exit()ing
  builtin/apply: make check_apply_state() return -1 instead of die()ing
  builtin/apply: move check_apply_state() to apply.c
  builtin/apply: make apply_all_patches() return 128 or 1 on error
  builtin/apply: make parse_traditional_patch() return -1 on error
  builtin/apply: make gitdiff_*() return 1 at end of header
  builtin/apply: make gitdiff_*() return -1 on error
  builtin/apply: change die_on_unsafe_path() to check_unsafe_path()
  builtin/apply: make build_fake_ancestor() return -1 on error
  builtin/apply: make remove_file() return -1 on error
  builtin/apply: make add_conflicted_stages_file() return -1 on error
  builtin/apply: make add_index_file() return -1 on error
  builtin/apply: make create_file() return -1 on error
  builtin/apply: make write_out_one_result() return -1 on error
  builtin/apply: make write_out_results() return -1 on error
  builtin/apply: make try_create_file() return -1 on error
  builtin/apply: make create_one_file() return -1 on error
  builtin/apply: rename option parsing functions
  apply: rename and move opt constants to apply.h
  Move libified code from builtin/apply.c to apply.{c,h}
  apply: make some parsing functions static again
  environment: add set_index_file()
  write_or_die: use warning() instead of fprintf(stderr, ...)
  apply: add 'be_silent' variable to 'struct apply_state'
  apply: make 'be_silent' incompatible with 'apply_verbosely'
  apply: don't print on stdout when be_silent is set
  usage: add set_warn_routine()
  usage: add get_error_routine() and get_warn_routine()
  apply: change error_routine when be_silent is set
  builtin/am: use apply api in run_apply()
  apply: use error_errno() where possible
I finally found time to get back to finish reviewing this.

The early part up to and including "apply: make some parsing
functions static again" looked good and we could treat them as a
continuation of the earlier cc/apply-introduce-state topic, which
has been merged to the 'master' already.

I got an impression that the patches in the remainder of the series
were not as polished as the earlier ones, except for the last one,
which should be reordered and be part of the early preparation step
if possible.

Thanks.

Re: [PATCH v8 00/41] libify apply and use lib in am, part 2

From: Christian Couder <hidden>
Date: 2016-07-27 06:15:15

On Tue, Jul 26, 2016 at 11:18 PM, Junio C Hamano [off-list ref] wrote:
Christian Couder [off-list ref] writes:
quoted
Sorry if this patch series is still long. I can split it into two or
more series if it is prefered.
...
Christian Couder (41):
  apply: make some names more specific
  apply: move 'struct apply_state' to apply.h
  builtin/apply: make apply_patch() return -1 or -128 instead of
    die()ing
  builtin/apply: read_patch_file() return -1 instead of die()ing
  builtin/apply: make find_header() return -128 instead of die()ing
  builtin/apply: make parse_chunk() return a negative integer on error
  builtin/apply: make parse_single_patch() return -1 on error
  builtin/apply: make parse_whitespace_option() return -1 instead of
    die()ing
  builtin/apply: make parse_ignorewhitespace_option() return -1 instead
    of die()ing
  builtin/apply: move init_apply_state() to apply.c
  apply: make init_apply_state() return -1 instead of exit()ing
  builtin/apply: make check_apply_state() return -1 instead of die()ing
  builtin/apply: move check_apply_state() to apply.c
  builtin/apply: make apply_all_patches() return 128 or 1 on error
  builtin/apply: make parse_traditional_patch() return -1 on error
  builtin/apply: make gitdiff_*() return 1 at end of header
  builtin/apply: make gitdiff_*() return -1 on error
  builtin/apply: change die_on_unsafe_path() to check_unsafe_path()
  builtin/apply: make build_fake_ancestor() return -1 on error
  builtin/apply: make remove_file() return -1 on error
  builtin/apply: make add_conflicted_stages_file() return -1 on error
  builtin/apply: make add_index_file() return -1 on error
  builtin/apply: make create_file() return -1 on error
  builtin/apply: make write_out_one_result() return -1 on error
  builtin/apply: make write_out_results() return -1 on error
  builtin/apply: make try_create_file() return -1 on error
  builtin/apply: make create_one_file() return -1 on error
  builtin/apply: rename option parsing functions
  apply: rename and move opt constants to apply.h
  Move libified code from builtin/apply.c to apply.{c,h}
  apply: make some parsing functions static again
  environment: add set_index_file()
  write_or_die: use warning() instead of fprintf(stderr, ...)
  apply: add 'be_silent' variable to 'struct apply_state'
  apply: make 'be_silent' incompatible with 'apply_verbosely'
  apply: don't print on stdout when be_silent is set
  usage: add set_warn_routine()
  usage: add get_error_routine() and get_warn_routine()
  apply: change error_routine when be_silent is set
  builtin/am: use apply api in run_apply()
  apply: use error_errno() where possible
I finally found time to get back to finish reviewing this.
Great, thanks!
The early part up to and including "apply: make some parsing
functions static again" looked good and we could treat them as a
continuation of the earlier cc/apply-introduce-state topic, which
has been merged to the 'master' already.
Ok, is it ok for you to just pick up this early part, or do you prefer
me to resend it (maybe with the last patch on top of it)?
I got an impression that the patches in the remainder of the series
were not as polished as the earlier ones, except for the last one,
which should be reordered and be part of the early preparation step
if possible.
I can resend just the last patch rebased on top of the early part once
you have picked up the early part.

And yeah the remainder has not been reviewed much. I will rework it as
you suggest in your other emails about specific pathes, and then
resend it in a "part 3" patch series.

Thanks,
Christian.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help