Re: [PATCH 1/2] xen: sched: rtds refactor code

[Meng Xu <mengxu@cis.upenn.edu>]

On Fri, Jun 24, 2016 at 3:45 AM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
> On Thu, 2016-06-23 at 11:42 +0100, George Dunlap wrote:
>> On 22/06/16 17:16, Meng Xu wrote:
>> >
>> > I think he is trying to align those comments to make them start
>> > from
>> > the same column. I was confused at the reason at the very
>> > beginning.
>> > Then I pulled his repo and checked this change.
>> Right -- well neither you as a reviewer nor anyone in the future
>> looking
>> back at this changeset should have to try to guess what the purpose
>> was;
>> if he did want to align them, that's perfectly fine, it just needs a
>> brief mention in the changelog. :-)
>>
> Indeed. BTW, I don't recall if we discussed this alignment
> previously,neither, in case we did, what my position was back then :-P
>
> In any case, I (now) think that having these comments aligned on a
> per-struct base is just fine, and there really is no need to have _all_
> of them aligned, across all structs.

Agree..

>
> I don't have a super strong opinion on this, and I'd be fine with it,
> if Meng is. I just think it's not worth the effort (of patching,
> reviewing, checking in, etc.)

Hmm, I don't have a strong opinion on this either.
I agree with George's comments on the commit message. I think it
should make this code change easier to understand in the future.
(Well, the code change is not hard to understand. So the improvement
of the commit message is not that critical.)

Now it's a matter of perfection or not.
If we want a "better" commit, I don't mind reviewing a new version. I
don't think this patch has been committed yet. So the extra work is in
our side, if we want to resend the patch. The work is as follows:
1) Replace "refactor" with "clean-up" for the patch title.
2) Go through and enumerate the different clean-ups this patch does.
Explain the reason for each type of the clean-ups. For instance, why
remove the __ at the head of functions (as described in the cover
letter)

OK. We need an action.
How about: Tianyang send another version for the work listed above. I
will review it again?

Thanks,

Meng

-----------
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel