* [PATCH/GSoC 3/3] Nousage message in error
@ 2016-03-24 2:03 Diwas Joshi
2016-03-24 5:14 ` Pranit Bauva
2016-03-24 7:24 ` Pranit Bauva
0 siblings, 2 replies; 5+ messages in thread
From: Diwas Joshi @ 2016-03-24 2:03 UTC (permalink / raw
To: git; +Cc: Diwas Joshi
- 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.
---
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);
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH/GSoC 3/3] Nousage message in error
2016-03-24 2:03 [PATCH/GSoC 3/3] Nousage message in error Diwas Joshi
@ 2016-03-24 5:14 ` Pranit Bauva
2016-03-24 16:23 ` Junio C Hamano
2016-03-24 7:24 ` Pranit Bauva
1 sibling, 1 reply; 5+ messages in thread
From: Pranit Bauva @ 2016-03-24 5:14 UTC (permalink / raw
To: Diwas Joshi; +Cc: Git List
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.
> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH/GSoC 3/3] Nousage message in error
2016-03-24 2:03 [PATCH/GSoC 3/3] Nousage message in error Diwas Joshi
2016-03-24 5:14 ` Pranit Bauva
@ 2016-03-24 7:24 ` Pranit Bauva
1 sibling, 0 replies; 5+ messages in thread
From: Pranit Bauva @ 2016-03-24 7:24 UTC (permalink / raw
To: Diwas Joshi; +Cc: Git List
On Thu, Mar 24, 2016 at 7:33 AM, Diwas Joshi <dj.dij123@gmail.com> wrote:
> - 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.
I forgot to mention that this microproject is already picked up by
someone else. Try attempting another microproject from the list. The
thread for the discussion of this is :
http://thread.gmane.org/gmane.comp.version-control.git/289312/focus=289336
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH/GSoC 3/3] Nousage message in error
2016-03-24 5:14 ` Pranit Bauva
@ 2016-03-24 16:23 ` Junio C Hamano
2016-03-24 16:28 ` Pranit Bauva
0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2016-03-24 16:23 UTC (permalink / raw
To: Pranit Bauva; +Cc: Diwas Joshi, Git List
Pranit Bauva <pranit.bauva@gmail.com> writes:
> 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?"
It is OK for other projects to adopt a different convention, but The
project convention here is different.
We tend to write our log message like this in this order:
- Explain relevant behaviour and code structure in the current code
to refresh memory of readers to help them understand the next two
items better. This paragraph is optional and you see it only in
difficult patches.
- Desribe the problem. What the end user would do and what she
sees in response to it, why that is not a good outcome, and what
would be a better outcome. For a patch to only improve code,
replace "the end user" with "other codepaths that interact with
the code being changed".
- Explain the approach to implement a better outcome. This
paragraph is optional and you see it only in patches that
implement tricky solutions.
- Describe the solution, as if you are giving orders to the
maintainers to change the code this way and that way.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH/GSoC 3/3] Nousage message in error
2016-03-24 16:23 ` Junio C Hamano
@ 2016-03-24 16:28 ` Pranit Bauva
0 siblings, 0 replies; 5+ messages in thread
From: Pranit Bauva @ 2016-03-24 16:28 UTC (permalink / raw
To: Junio C Hamano; +Cc: Diwas Joshi, Git List
On Thu, Mar 24, 2016 at 9:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Pranit Bauva <pranit.bauva@gmail.com> writes:
>
>> 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?"
>
> It is OK for other projects to adopt a different convention, but The
> project convention here is different.
>
> We tend to write our log message like this in this order:
>
> - Explain relevant behaviour and code structure in the current code
> to refresh memory of readers to help them understand the next two
> items better. This paragraph is optional and you see it only in
> difficult patches.
>
> - Desribe the problem. What the end user would do and what she
> sees in response to it, why that is not a good outcome, and what
> would be a better outcome. For a patch to only improve code,
> replace "the end user" with "other codepaths that interact with
> the code being changed".
>
> - Explain the approach to implement a better outcome. This
> paragraph is optional and you see it only in patches that
> implement tricky solutions.
>
> - Describe the solution, as if you are giving orders to the
> maintainers to change the code this way and that way.
Thanks for making it clearer to me! I tend to write bad commit
messages. Improving on it though.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-03-24 16:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-24 2:03 [PATCH/GSoC 3/3] Nousage message in error Diwas Joshi
2016-03-24 5:14 ` Pranit Bauva
2016-03-24 16:23 ` Junio C Hamano
2016-03-24 16:28 ` Pranit Bauva
2016-03-24 7:24 ` Pranit Bauva
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).