Re: [PATCH 0/2] Submodule merging: i18n, verbosity

[Stefan Beller]

Hi Elijah,

On Thu, May 10, 2018 at 5:04 PM, Elijah Newren  wrote:
> On Thu, May 10, 2018 at 2:19 PM, Stefan Beller  wrote:
>> Leif wrote:
>>> Sure, let me know what to use instead and I’ll update and resubmit the 
>>> patch.
>>> Sure, but `MERGE_WARNING` prefixes all the messages with "Failed to
>>> merge submodule“.
>>
>> I thought about replying and coming up with good reasons, but I wrote some
>> patches instead.
>>
>> They can also be found at 
>> https://github.com/stefanbeller/git/tree/submodule_i18n_verbose
>>
>> I think these would be a good foundation for your patch as well, as you can 
>> use the
>> output() function for the desired cases.
>>
>> Feel free to take these patches as part of your series or adapt
>> (or be inspired by) as needed.
>
> This is awesome.  In addition to the good reasons you gave, switching
> merge_submodule() to use output() was one of several things on my todo
> list since I think it'd be needed for remerge-diffs
> (https://bugs.chromium.org/p/git/issues/detail?id=12) and might be
> useful for merges in bare repos; thanks for tackling it.

Thanks for the encouraging words!
The one nit I find on that series is that we need to rely on and export
the add_submodule_odb function as I want to get rid of that function
once the object store series has progressed far enough.

>
> Patches look good to me.  Having Leif's patch on top of these two
> would be great.

ok, Let's go with that.

Stefan

Re: [PATCH 0/2] Submodule merging: i18n, verbosity

[Elijah Newren]

On Thu, May 10, 2018 at 2:19 PM, Stefan Beller  wrote:
> Leif wrote:
>> Sure, let me know what to use instead and I’ll update and resubmit the patch.
>> Sure, but `MERGE_WARNING` prefixes all the messages with "Failed to
>> merge submodule“.
>
> I thought about replying and coming up with good reasons, but I wrote some
> patches instead.
>
> They can also be found at 
> https://github.com/stefanbeller/git/tree/submodule_i18n_verbose
>
> I think these would be a good foundation for your patch as well, as you can 
> use the
> output() function for the desired cases.
>
> Feel free to take these patches as part of your series or adapt
> (or be inspired by) as needed.

This is awesome.  In addition to the good reasons you gave, switching
merge_submodule() to use output() was one of several things on my todo
list since I think it'd be needed for remerge-diffs
(https://bugs.chromium.org/p/git/issues/detail?id=12) and might be
useful for merges in bare repos; thanks for tackling it.

Patches look good to me.  Having Leif's patch on top of these two
would be great.

Elijah

[PATCH 0/2] Submodule merging: i18n, verbosity

[Stefan Beller]

Leif wrote:
> Sure, let me know what to use instead and I’ll update and resubmit the patch.
> Sure, but `MERGE_WARNING` prefixes all the messages with "Failed to
> merge submodule“.

I thought about replying and coming up with good reasons, but I wrote some
patches instead.

They can also be found at 
https://github.com/stefanbeller/git/tree/submodule_i18n_verbose

I think these would be a good foundation for your patch as well, as you can use 
the
output() function for the desired cases.

Feel free to take these patches as part of your series or adapt
(or be inspired by) as needed.

Thanks,
Stefan


Stefan Beller (2):
  submodule.c: move submodule merging to merge-recursive.c
  merge-recursive: i18n submodule merge output and respect verbosity

 merge-recursive.c | 169 +-
 submodule.c   | 168 +
 submodule.h   |   6 +-
 3 files changed, 170 insertions(+), 173 deletions(-)

-- 
2.17.0.255.g8bfb7c0704