Stefan Beller writes:
> @@ -41,18 +35,16 @@ static void remove_temporary_files(void)
> DIR *dir;
> struct dirent *e;
>
> + dir = opendir(packdir);
> + if (!dir)
> return;
>
> + strbuf_addstr(&buf, packdir);
> +
> + /* dirlen holds the length of the pa
Am 21.08.2013 15:07, schrieb Matthieu Moy:
Stefan Beller writes:
But as these follow up changes heavily rely on the very first patch
I will first try to get that right, meaning accepted into pu.
Then I can send patches with these proposals such as making more
functions.
I think it's better t
Am 21.08.2013 10:49, schrieb Matthieu Moy:
Stefan Beller writes:
+ for_each_string_list_item(item, &names) {
+ for (ext = 0; ext < 2; ext++) {
+ char *fname, *fname_old;
+ fname = mkpathdup("%s/%s%s", packdir, item->string,
exts[e
On 08/21/2013 10:25 AM, Jonathan Nieder wrote:
> Hi,
>
> Stefan Beller wrote:
>
>> [PATCH] repack: rewrite the shell script in C.
>
> Thanks for your work so far. This review will have mostly cosmetic
> notes. Hopefully others can try it out to see if the actual behavior
> is good.
Thanks for
Stefan Beller writes:
> But as these follow up changes heavily rely on the very first patch
> I will first try to get that right, meaning accepted into pu.
> Then I can send patches with these proposals such as making more
> functions.
I think it's better to get the style right before, to avoid
Stefan Beller writes:
> On 08/21/2013 10:49 AM, Matthieu Moy wrote:
>>> + if (start_command(&cmd))
>>> > + return 1;
>> A warning message would be welcome in addition to returning 1.
>>
>
> Johannes Sixt proposes to retain the return value of
> the sub process, which I'd agree on.
Yes
On 08/21/2013 10:49 AM, Matthieu Moy wrote:
> I tend to dislike these "set a variable and break twice" to exit nested
> loops. Using an auxiliary function, you could just do
>
> int f()
> {
> for_each {
> for () {
> ...
> if ()
>
On 08/21/2013 10:49 AM, Matthieu Moy wrote:
>> +if (start_command(&cmd))
>> > + return 1;
> A warning message would be welcome in addition to returning 1.
>
Johannes Sixt proposes to retain the return value of
the sub process, which I'd agree on.
However why do we need a warning mess
On 08/21/2013 10:25 AM, Jonathan Nieder wrote:
>> +static int delta_base_offset = 0;
>
> The "= 0" is automatic for statics without an initializer. The
> prevailing style in git is to leave it out.
>
> Behavior change: in the script, wasn't the default "true"?
>
Yes, I was printing out the arg
Stefan Beller writes:
> All tests are constantly positive now.
Cool!
> +/*
> + * Fills the filename list with all the files found in the pack directory
Detail: "filename list" could be "fname_list" to match the actual
argument below.
> + * ending with .pack, without that extension.
> + */
> +
Hi,
Stefan Beller wrote:
> [PATCH] repack: rewrite the shell script in C.
Thanks for your work so far. This review will have mostly cosmetic
notes. Hopefully others can try it out to see if the actual behavior
is good.
As a first nit: in git, as usual in emails, the style in subject lines
is
On 08/14/2013 09:26 AM, Matthieu Moy wrote:
> I suggested that you first enrich the test suite if needed. Did you
> check that the testsuite had good enough coverage for git-repack?
There are already quite some tests using git-repack for testing other
purposes. Also git repack seems to be called f
Stefan Beller writes:
> This is the beginning of the rewrite of the repacking.
(please, mark your patch as RFC/PATCH in the subject in this case)
A few quick comments on style below.
> Makefile | 2 +-
> builtin.h | 1 +
> builtin/repack.c
13 matches
Mail list logo
