Re: [PATCH/GSoC 3/3] Nousage message in error
- From
Pranit Bauva <pranit.bauva@gmail.com>
- Date
- Mar 24, 2016, 05:14 UTC
- Message-ID
- <CAFZEwPN2vp+zOMdGY51LwNpgNcYxsGD4GBXHNOA8_qJ8vtE5OQ@mail.gmail.com>
- In-Reply-To
- <1458785018-29232-1-git-send-email-dj.dij123@gmail.com>
This is my first review. It can contain some mistakes.
On Thu, Mar 24, 2016 at 7:33 AM, Diwas Joshi <dj.dij123@gmail.com> wrote:
> Subject : [PATCH/GSoC 3/3] Nousage message in error
Mention about GSoC in the notes section (the one followed by the 3 dashes ie. "---") rather than in the subject.
> - To show only error text instead of full usage message > - Adds exits to callback function in parse-options-cb.c instead of returning -1 which results in display of usage message.
A general convention followed by git users it to write the commit message as "What he did to the code?" rather than "What problem was there in the code?" And of course after writing what you did to the code, you can definitely mention what problem in the code made you do this change.
Show 20 quoted lines
> parse-options-cb.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/parse-options-cb.c b/parse-options-cb.c
> index 239898d..b7321d1 100644
> --- a/parse-options-cb.c
> +++ b/parse-options-cb.c
> @@ -85,8 +85,10 @@ int parse_opt_commits(const struct option *opt, const char *arg, int unset)
>
> if (!arg)
> return -1;
> - if (get_sha1(arg, sha1))
> - return error("malformed object name %s", arg);
> + if (get_sha1(arg, sha1)) {
> + error("malformed object name %s", arg);
> + exit(129);
> + }
> commit = lookup_commit_reference(sha1);
> if (!commit)
> return error("no such commit %s", arg);Maybe you could describe a little more on why this change is required? Why would the user want to know "How to use the command?" when the actual problem is that SHA-1 checksum has been compromised? And I don't see any consumers of this method which *directly* interact with the UI.
It seems that PATCH 1/3 and PATCH 2/3 are missing.