Re: [Xen-devel] [PATCH V2 0/9] xentrace/xenalyze support on ARM

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Apr 04, 2016 at 03:50:35PM +0000, Ben Sanda wrote:
> Julien and Wei,
> 
> >> You patches all have the same subject line.  Please make them more 
> >> specific. See my reply to #1 for example.
> >
> > +1
> > 
> > Also, you should at least check that Xen still builds after applying
> > each patch. Ideally, you also need to be careful to not break any
> > feature currently supported. It's useful when someone needs to 
> > bisect the tree.
> >
> > For instance, you use the function get_pg_owner for ARM in patch #2 
> > but introduce the function in patch #4. This will break ARM build. 
> > So the patch #2 should be moved after #4.
> >
> > Furthermore, you remove the functions get_pg_owner and put_pg_owner 
> > for x86 in patch #3 and then re-introduced them in patch #4. 
> > Therefore, the x86 will be broken after #3. In this case, it's better
> > to have a patch that move the 2 functions from x86 to common.
> 
> Thank you for the comments. I apologize for the errors in the patch
> format. This is my first time submitting a patch to Xen and I was
> unaware that the patch set order mattered or that I had to account for
> a piecewise application of the patch set. I will attempt to resubmit
> with this corrected and the patch names updated. 
> 
> So then it is permissible to have multiple file changes in one
> patch/commit? E.g. a patch that removes from one file and  adds to

That's definitely allowed. Just think of each commit as a logically
complete unit. It should compile. It should not break existing
functionality.

Wei.

> another in the same commit. I initially thought each patch/ commit was
> only supposed to modify one file and that's why I did it that way
> 
> Thank you,
> Ben Sanda

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