On 25-10-2021 18:07, Ævar Arnfjörð Bjarmason wrote:
On Mon, Oct 25 2021, Abhradeep Chakraborty via GitGitGadget wrote:From: Abhradeep Chakraborty <chakrabortyabhradeep79@xxxxxxxxx> Issue no #635 in the gitgitgadget/git repo has a brief description about the violation of error message style convention. These violations are - 1) Do not end error messages with a full stop. 2) Do not capitalize ("unable to open %s", not "Unable to open %s") 3) Say what the error is first ("cannot open %s", not "%s: cannot open")Let's summarize the guidelines instead of asking readers to visit https://github.com/gitgitgadget/git/issues/635 (which isn't even directly linked here) only to find a quote from CodingGuidelines.
sorry for not mentioning that. I didn't realize at the time of writing the commit message.
I think it's good to split these up, e.g. the rewording of the BUG() message for the 3rd quoted item in CodingGuidelines from cases where you're only changing the capitalization of these messages.
But both of these don't comply with the error message style convention. So, don't you think that all should be in the same commit (all of them are equally violating style conventions)?
If we're going to mass-edit these I wouldn't mind an addition to the guidelines that says we ''-quote things, and add that when appropriate....
Pardon me, I didn't understand the "we ''-quote things, and add that when appropriate" part. Could you please explain that again?
- error(_("You have not concluded your merge (MERGE_HEAD exists).")); + error(_("you have not concluded your merge (MERGE_HEAD exists)")); if (advice_enabled(ADVICE_RESOLVE_CONFLICT)) advise(_("Please, commit your changes before merging.")); - die(_("Exiting because of unfinished merge.")); + die(_("exiting because of unfinished merge")); }Surely any convention of not using full-stop should extend to advise() too?
I was thinking of it and even was confused at the starting. But later I thought
that advise and errors are not the same thing. So, there shouldn't be any change
for advise(). Though I am not so confident about this.
But in this case aren't we constructing sentences in parts? Maybe having the full-stops here is better than not, I'm not sure.
Maybe you're right. One question to ask is, do error() and advise() add "\n" at the end of these strings?
Also, have you tried using coccinelle for this? Doing this sort of replacement should be a small matter of some inline Python, the linux.git repo has some good examples of that. It would make discussing these changes easier, and per the "pending" docs in contrib/coccinelle/README any patches could follow such rules incrementally...
No, I haven't used coccinelle but will definitely take a look at it. Thanks.
