Re: [PATCH] repack: Add --version parameter
On 09/26/2013 12:17 PM, Felipe Contreras wrote:
On Thu, Sep 26, 2013 at 3:32 AM, Stefan Beller
static const char *const git_repack_usage[] = {
@@ -22,6 +23,9 @@ static int repack_config(const char *var, const char
*value, void *cb)
delta_base_offset = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, core.preferredPackVersion)) {
+ pack_version = git_config_int(var, value);
+ }
The style is without braces if the block is a single line.
Thanks for spotting, I should have put a return 0; there.
Stefan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] repack: Add --version parameter
On 09/26/2013 01:42 PM, Duy Nguyen wrote:
On Thu, Sep 26, 2013 at 3:32 PM, Stefan Beller
stefanbel...@googlemail.com wrote:
This is just a direct translation of
http://article.gmane.org/gmane.comp.version-control.git/235396
So I don't consider this is ready for inclusion.
Some notes:
We need to have more error checking, repack shall be 0, 2 or 4 but
nothing
else. If 0 is given, no argument is passed to pack-objects, in case of
2 or 4 --version=n is passed.
It's not that bad. If you don't catch it, pack-objects will.
Ok, noted.
Do we really want to call it --version? This parameter sounds so much
like questioning for the program version, similar to
git --version
1.8.4
So I'd rather use --repack-version.
Hmm.. I think it's git repack --pack-version? Or if you meant git
pack-objects --version, I drop the pack- out because there's
already pack in pack-objects. But I'm OK renaming --version to
--pack-version too. Maybe later.
@@ -22,6 +23,9 @@ static int repack_config(const char *var, const
char *value, void *cb)
delta_base_offset = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, core.preferredPackVersion)) {
+ pack_version = git_config_int(var, value);
+ }
return git_default_config(var, value, cb);
In np/pack-v4 series (not the one on 'pu' yet) git_default_config will
do this and save the value in core_default_pack_format. So you don't
need to set it here.
@@ -220,6 +226,8 @@ int cmd_repack(int argc, const char **argv, const
char *prefix)
argv_array_push(cmd_args, --quiet);
if (delta_base_offset)
argv_array_push(cmd_args, --delta-base-offset);
+ if (pack_version)
+ argv_array_pushf(cmd_args, --version=%u,
pack_version);
but then you may need if (!pack_version) pack_version =
core_defaul_pack_format; before this if.
The reason I put the pack_version not here is for structural clarity.
(All config is done in either the parse_options section or in the
repack_config function). This may help having a the actual core logic
easier and more understandable?
If you feel otherwise, I'd change it to your proposal.
Thanks,
Stefan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] repack: Add --version parameter
On Thu, Sep 26, 2013 at 3:32 AM, Stefan Beller
stefanbel...@googlemail.com wrote:
This is just a direct translation of
http://article.gmane.org/gmane.comp.version-control.git/235396
So I don't consider this is ready for inclusion.
Some notes:
We need to have more error checking, repack shall be 0, 2 or 4 but nothing
else. If 0 is given, no argument is passed to pack-objects, in case of
2 or 4 --version=n is passed.
Do we really want to call it --version? This parameter sounds so much
like questioning for the program version, similar to
git --version
1.8.4
So I'd rather use --repack-version.
---
builtin/repack.c | 8
1 file changed, 8 insertions(+)
diff --git a/builtin/repack.c b/builtin/repack.c
index 3e56614..fd05e9a 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -9,6 +9,7 @@
#include argv-array.h
static int delta_base_offset = 1;
+static int pack_version;
static char *packdir, *packtmp;
static const char *const git_repack_usage[] = {
@@ -22,6 +23,9 @@ static int repack_config(const char *var, const char
*value, void *cb)
delta_base_offset = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, core.preferredPackVersion)) {
+ pack_version = git_config_int(var, value);
+ }
The style is without braces if the block is a single line.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] repack: Add --version parameter
On Thu, Sep 26, 2013 at 3:32 PM, Stefan Beller
stefanbel...@googlemail.com wrote:
This is just a direct translation of
http://article.gmane.org/gmane.comp.version-control.git/235396
So I don't consider this is ready for inclusion.
Some notes:
We need to have more error checking, repack shall be 0, 2 or 4 but nothing
else. If 0 is given, no argument is passed to pack-objects, in case of
2 or 4 --version=n is passed.
It's not that bad. If you don't catch it, pack-objects will.
Do we really want to call it --version? This parameter sounds so much
like questioning for the program version, similar to
git --version
1.8.4
So I'd rather use --repack-version.
Hmm.. I think it's git repack --pack-version? Or if you meant git
pack-objects --version, I drop the pack- out because there's
already pack in pack-objects. But I'm OK renaming --version to
--pack-version too. Maybe later.
@@ -22,6 +23,9 @@ static int repack_config(const char *var, const char
*value, void *cb)
delta_base_offset = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, core.preferredPackVersion)) {
+ pack_version = git_config_int(var, value);
+ }
return git_default_config(var, value, cb);
In np/pack-v4 series (not the one on 'pu' yet) git_default_config will
do this and save the value in core_default_pack_format. So you don't
need to set it here.
@@ -220,6 +226,8 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
argv_array_push(cmd_args, --quiet);
if (delta_base_offset)
argv_array_push(cmd_args, --delta-base-offset);
+ if (pack_version)
+ argv_array_pushf(cmd_args, --version=%u, pack_version);
but then you may need if (!pack_version) pack_version =
core_defaul_pack_format; before this if.
argv_array_push(cmd_args, packtmp);
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
