From: work <motroniii@gmail.com>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/2] Modified flag field type in rev_list_info struct in bisect.h. There is no need for flag field to be signed, as it is not supposed to be used as decimal.
Date: Thu, 24 Mar 2016 23:01:12 +0300 [thread overview]
Message-ID: <56F44788.2050509@gmail.com> (raw)
In-Reply-To: <CAGZ79kZmzo+f9YF1K1oC2GfOrzdaojmrca7bUm3cBRoDreeA-g@mail.gmail.com>
On 03/24/2016 10:51 PM, Stefan Beller wrote:
> On Thu, Mar 24, 2016 at 12:41 PM, Motroni Igor <motroniii@gmail.com> wrote:
>> From: Pontifik <motroniii@gmail.com>
> Here is a good place to put reasoning for why this is a good idea.
> I see you have a long subject, so maybe we can shorten the first line
> (down to less than ~ 80 characters) and put the longer explanation
> here.
>
> How about:
>
> bisect: use unsigned for flag field
>
> The flags are usually used as a unsigned variable, because it makes
> bit operations easier to follow.
Yep, it's definitely a good idea to shorten subject in order to put more
explanations in body of a message.
>
>
>> Signed-off-by: Pontifik <motroniii@gmail.com>
> From Documentation/SubmittingPatches:
>> Also notice that a real name is used in the Signed-off-by: line. Please
>> don't hide your real name.
Yep, I didn't mean to hide my real name, but coming across lots of new
stuff confused me in distinguishing whether I'm asked to enter real name
or nickname :)
>> ---
>> bisect.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/bisect.h b/bisect.h
>> index acd12ef..a979a7f 100644
>> --- a/bisect.h
>> +++ b/bisect.h
>> @@ -16,7 +16,7 @@ extern struct commit_list *filter_skipped(struct commit_list *list,
>>
>> struct rev_list_info {
>> struct rev_info *revs;
>> - int flags;
>> + unsigned int flags;
> You can also drop the int here and make it just
> unsigned.
In fact, I just wanted to keep this code clear to understand (as I see
this). Unsigned short is also unsigned, but a reader should know that
"unsigned" type stands for "unsigned int". Anyway, I'll keep this in
mind in future, thanks a lot.
>> int show_timestamp;
>> int hdr_termination;
>> const char *header_prefix;
>> --
>> 2.5.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2016-03-24 20:01 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-24 19:41 [PATCH 0/2] GSoC student Motroni Igor
2016-03-24 19:41 ` [PATCH 1/2] Modified flag field type in rev_list_info struct in bisect.h. There is no need for flag field to be signed, as it is not supposed to be used as decimal Motroni Igor
2016-03-24 19:51 ` Stefan Beller
2016-03-24 20:01 ` work [this message]
2016-03-24 22:56 ` Eric Sunshine
2016-03-24 23:04 ` Stefan Beller
2016-03-25 6:18 ` Junio C Hamano
2016-03-25 12:53 ` work
2016-03-25 16:54 ` Eric Sunshine
2016-03-24 19:41 ` [PATCH 2/2] Just a minor commit to trigger Travis Ci build Motroni Igor
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=56F44788.2050509@gmail.com \
--to=motroniii@gmail.com \
--cc=git@vger.kernel.org \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).