Re: [PATCH v5 01/17] rbtree: changes to align the coding conventions with Linux tree

From: Praveen Kumar <kpraveen.lkml@gmail.com>
To: Dario Faggioli <dario.faggioli@citrix.com>
Cc: tim@xen.org, sstabellini@kernel.org, wei.liu2@citrix.com,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	ian.jackson@eu.citrix.com, xen-devel@lists.xen.org,
	Jan Beulich <JBeulich@suse.com>
Subject: Re: [PATCH v5 01/17] rbtree: changes to align the coding conventions with Linux tree
Date: Fri, 4 Aug 2017 22:29:28 +0530	[thread overview]
Message-ID: <CABcWhv43P-QvJBvqSE3cFFBYn+E_zX3oi1TbR7OMoJe4m2-4pw@mail.gmail.com> (raw)
In-Reply-To: <1501756670.28477.6.camel@citrix.com>

Hi Dario,

On Thu, Aug 3, 2017 at 4:07 PM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
> On Fri, 2017-07-14 at 07:05 -0600, Jan Beulich wrote:
>> > > > On 14.07.17 at 14:51, <kpraveen.lkml@gmail.com> wrote:
>> >
>> > Agreed, I shouldn't have added.
>> > rbtree.h file does include incline functions which are actually
>> > commented, and in order to have complete similarity I did include
>> > the
>> > same here.
>> >
>> > Also, rbtree.c does have comment in header note being modified, for
>> > the
>> > same reason.
>> >
>> > Further, do you suggest to keep the old ones, but that may cause
>> > porting issue and it won't be exact replica from Linux base. Please
>> > suggest.
>>
>> I'm fine with comment updates, _as long as you say so_ in the
>> commit message. If you say "only style changes", then there
>> ought to be no additions whatsoever.
>>
> I fully agree with Jan.
>
> And, as him, I also think you can update the header comments at the
> beginning of both rbtree.c and rbtree.h files, as soon as you mention
> that in the changelog.
>

Sure, will try to update with each patch individually in the header
comments ( if there are any version change ).

> *HOWEVER*, about this change, in both .c and .h:
>
> @@ -14,7 +14,8 @@
>    GNU General Public License for more details.
>
>    You should have received a copy of the GNU General Public License
> -  along with this program; If not, see <http://www.gnu.org/licenses/>.
> +  along with this program; if not, write to the Free Software
> +  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>
>    linux/lib/rbtree.c
>  */
>
> This comes from 443701ef "Replace FSF street address with canonical
> URL" (check with `git blame xen/common/rbtree.c'), and I think we
> should leave this alone (i.e., keep the url, and not change back to the
> physical address).
>
> I understand it then will be a difference between our rbtree.{c,h} and
> Linux's ones, but I think it's one difference it's worth living with
> (and, honestly, I really don't expect this specific thing to cause much
> issues in future 'backports' from Linux).
>
> If others agree on this too, that would mean you basically would let
> the header comment of rbtree.c alone, while in rbtree.h, you "just" add
> the commented API usage example functions.
>

There will be issue while directly applying the patch from Linux tree
( having changed the file name ) as the line number changes. Because
of which I included the commented code. Further to this, for some of
the patches shared, I was also facing porting issue with and has been
manually ported. So, I am thinking if maintaining complete accuracy /
replica with Linux tree will give any benefit ?

> Regards,
> Dario
> --
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

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