Guillaume Pagès <guillaume.pages@xxxxxxxxxxxxxxxxxxxxxxx> writes:
> It is an almost empty code I send to validate the global architecture
> of this command. I choose to write
Avoid personal wording (I choose to write ... because -> The code is
written ... because). What matters in the commit message is the reason
for the choice, not who made it.
> +BUILTIN_OBJS += builtin/rebase--status--helper.o
No builtin/rebase--status--helper.c in your patch, is it intended?
> +status)
> + git rebase--status--helper
> + die
"die" is used to exit with an error code ($? != 0). You just mean exit
$? here, to exit with the same code as the helper.
And actually, you don't need to keep your script alive while the helper
is ran, so you can write
exec git rebase--status--helper
> --- /dev/null
> +++ b/rebase--status.c
> @@ -0,0 +1,6 @@
> +#include "rebase--status.h"
> +
> +int rebase_status(){
> + printf("Rebase in progress\n");
... or not.
Avoid leaving incorrect things like this in intermediate steps, even if
you're going to fix them eventually. It's too easy to forget the actual
fix and leave a "Rebase in progress" message even when there's no rebase
in progress. And reviewers may get confused.
I'd have written stg like
printf("git rebase --status is not yet implemented");
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html