Re: Improving code base readability

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

Alangi Derick wrote:

> This is an example of what i am talking about or what i am trying to
> demonstrate. This is the patch:

Thanks for giving an example.

[...]
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -102,7 +102,7 @@ static int stream_blocked(const unsigned char *sha1)
> 
>   st = open_istream(sha1, &type, &sz, NULL);
>   if (!st)
> - return error("cannot stream blob %s", sha1_to_hex(sha1));
> + return error("Cannot stream blob %s", sha1_to_hex(sha1));

This is not about code readability but about the program's output, no?

Habit is to use lowercase for error messages, to be brief, and to not
include a period at the end since they are not sentences.  For
example:

	$ nonexistent-command
	bash: nonexistent-command: command not found

Git tends to follow that practice, too, though it is not completely
consistent about it.

As a first contribution, I don't think this is a good project to
embark on.  It would touch a lot of code, meaning it is difficult to
time to avoid interfering with other patches.  It is relatively
low-impact or the impact is hard to demonstrate: consistency is
pleasant, but the capitalization does not seem to be interfering with
the program's usability.  Once the work is done, it is easy to
backslide unless there is an automated test to avoid that, which makes
the work more technically complicated.

If you'd like a project that involves touching a large part of the
codebase, one ongoing project has been to mark human-readable strings
for translation.  See "Marking strings for translation" in po/README
for hints about how that works.

The same timing issues about avoiding conflicting with other people's
work apply there, too, but I'd be happy to help as you go.  And the
impact can be pretty big, both in consistency (a mixture of English
and native language output due to incomplete i18n is not a great
experience) and usability (for some, being able to interact with git
in their native language makes using git much easier).

A search with

	git grep --cached -F -e 'error("'

finds many files with untranslated strings.

Such work might lead you to discover unclear code, unclear messages,
or aspects of git's behavior that you'd like to change, which can also
lead to other patches that go in other directions.

What do you think?

Thanks,
Jonathan