[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/2] xen: sched: rtds refactor code
On Fri, Jun 24, 2016 at 3:45 AM, Dario Faggioli <dario.faggioli@xxxxxxxxxx> 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@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |