From: Thomas Rast <tr@thomasrast•ch>
To: Akshay Aurora <akshayaurora@yahoo•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] Rewrite diff-no-index.c:read_directory() to use is_dot_or_dotdot() and rename it to read_dir()
Date: Sun, 16 Mar 2014 12:07:20 +0100 [thread overview]
Message-ID: <87eh22xuzb.fsf@thomasrast.ch> (raw)
In-Reply-To: <CAPGnZZn_Wz=LywVevmuXWQX4nO67EKMPazB8jKv-jTZ178=HdQ@mail.gmail.com> (Akshay Aurora's message of "Sat, 15 Mar 2014 12:14:18 +0530")
Akshay Aurora <akshayaurora@yahoo•com> writes:
> Forgot to mention, this is one of the microprojects for GSoC this
> year. Would be great to have some feedback.
>
> On Fri, Mar 14, 2014 at 6:09 PM, Akshay Aurora <akshayaurora@yahoo•com> wrote:
>> I have renamed diff-no-index.c:read_directory() to read_dir() to avoid name collision with dir.c:read_directory()
>>
>> Signed-off-by: Akshay Aurora <akshayaurora@yahoo•com>
Hmm, the original mail never made it through to me, and gmane doesn't
seem to have it either. What happened here? The headers suggest you
used git-send-email, which should avoid these problems. Can you dig up
the command and configuration you used to send it (but be careful to not
post your password!)?
On the patch itself:
> Subject: Re: [PATCH] Rewrite diff-no-index.c:read_directory() to use is_dot_or_dotdot() and rename it to read_dir()
The subject line is very long. Aim for 50 characters, but certainly no
more than 72.
You are also conflating two separate things into one patch. Try to
avoid doing that.
Furthermore I am unconvinced that renaming a function from
read_directory() to read_dir() is a win. What are you trying to improve
by the rename? Renames are good if they improve clarity and/or
consistency, but read_dir() just risks confusion with readdir() and I
cannot see any gain in consistency that would compensate for it.
>> I have renamed diff-no-index.c:read_directory() to read_dir() to avoid name collision with dir.c:read_directory()
Please stick to the style outlined in SubmittingPatches:
The body should provide a meaningful commit message, which:
. explains the problem the change tries to solve, iow, what is wrong
with the current code without the change.
. justifies the way the change solves the problem, iow, why the
result with the change is better.
. alternate solutions considered but discarded, if any.
Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
to do frotz", as if you are giving orders to the codebase to change
its behaviour. Try to make sure your explanation can be understood
without external resources. Instead of giving a URL to a mailing list
archive, summarize the relevant points of the discussion.
Also, please wrap your commit messages at 72 characters.
>> ---
>> diff-no-index.c | 9 +++++----
The microproject idea said
Rewrite diff-no-index.c:read_directory() to use
is_dot_or_dotdot(). Try to find other sites that can use that
function.
Are there any others?
--
Thomas Rast
tr@thomasrast•ch
prev parent reply other threads:[~2014-03-16 11:07 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1394800759-87648-1-git-send-email-akshayaurora@yahoo.com>
2014-03-15 6:44 ` [PATCH] Rewrite diff-no-index.c:read_directory() to use is_dot_or_dotdot() and rename it to read_dir() Akshay Aurora
2014-03-16 11:07 ` Thomas Rast [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87eh22xuzb.fsf@thomasrast.ch \
--to=tr@thomasrast$(echo .)ch \
--cc=akshayaurora@yahoo$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox