[LTP] [PATCH 1/1] statx04: Remove kernel comments in docparse

[LTP] [PATCH 1/1] statx04: Remove kernel comments in docparse

[Petr Vorel]

They are defined in .tags, having in docparse results into poor
formating in metadata.{html,pdf}.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Hi Li,

I've done cleanup like this in the past, but sending a patch just to
make consensus about it. If we prefer to have git commits like this in
the code (i.e. if being in .tags is not enough), they should be in
normal comments /* ... */ so that they aren't in docparse.

IMHO docparse can mention some commit if wanted to add some description,
but just as 5f955f26f3d4 or 5f955f26f3d4 ("xfs: report crtime and
attribute flags to statx") if the commit subject is a description
itself.

http://linux-test-project.github.io/metadata/metadata.stable.html#statx04

Kind regards,
Petr

 testcases/kernel/syscalls/statx/statx04.c | 35 +----------------------
 1 file changed, 1 insertion(+), 34 deletions(-)

diff --git a/testcases/kernel/syscalls/statx/statx04.c b/testcases/kernel/syscalls/statx/statx04.c
index 98f9a6315..6c562b3d7 100644
--- a/testcases/kernel/syscalls/statx/statx04.c
+++ b/testcases/kernel/syscalls/statx/statx04.c
@@ -14,41 +14,8 @@
  * - STATX_ATTR_NODUMP: File is not a candidate for backup when a backup
  *                        program such as dump(8) is run.
  *
- * xfs filesystem doesn't support STATX_ATTR_COMPRESSED flag, so we only test
+ * XFS filesystem doesn't support STATX_ATTR_COMPRESSED flag, so we only test
  * three other flags.
- *
- * ext2, ext4, btrfs, xfs and tmpfs support statx syscall since the following commit
- *
- *  commit 93bc420ed41df63a18ae794101f7cbf45226a6ef
- *  Author: yangerkun <yangerkun@huawei.com>
- *  Date:   Mon Feb 18 09:07:02 2019 +0800
- *
- *  ext2: support statx syscall
- *
- *  commit 99652ea56a4186bc5bf8a3721c5353f41b35ebcb
- *  Author: David Howells <dhowells@redhat.com>
- *  Date:   Fri Mar 31 18:31:56 2017 +0100
- *
- *  ext4: Add statx support
- *
- *  commit 04a87e3472828f769a93655d7c64a27573bdbc2c
- *  Author: Yonghong Song <yhs@fb.com>
- *  Date:   Fri May 12 15:07:43 2017 -0700
- *
- *  Btrfs: add statx support
- *
- *  commit 5f955f26f3d42d04aba65590a32eb70eedb7f37d
- *  Author: Darrick J. Wong <darrick.wong@oracle.com>
- *  Date:   Fri Mar 31 18:32:03 2017 +0100
- *
- *  xfs: report crtime and attribute flags to statx
- *
- *  commit e408e695f5f1f60d784913afc45ff2c387a5aeb8
- *  Author: Theodore Ts'o <tytso@mit.edu>
- *  Date:   Thu Jul 14 21:59:12 2022 -0400
- *
- *  mm/shmem: support FS_IOC_[SG]ETFLAGS in tmpfs
- *
  */
 
 #define _GNU_SOURCE
-- 
2.37.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] statx04: Remove kernel comments in docparse

[xuyang2018.jy]

Hi Petr

Oh, I see commit title in metadata that I don't see it before.

Tag	   Info
linux-git 93bc420ed41d ("ext2: support statx syscall")

linux-git 99652ea56a41 ("ext4: Add statx support")

linux-git 04a87e347282 ("Btrfs: add statx support")

linux-git 5f955f26f3d4 ("xfs: report crtime and attribute flags to statx")


But user that doesn't use metadata(miss this usage or miss dependent 
package ie rubygem-asciidoctor), then they only see some numbers in tag 
but know nothing.

IMO, it is not clear like min_kever.

Best Regards
Yang Xu
> They are defined in .tags, having in docparse results into poor
> formating in metadata.{html,pdf}.
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Hi Li,
> 
> I've done cleanup like this in the past, but sending a patch just to
> make consensus about it. If we prefer to have git commits like this in
> the code (i.e. if being in .tags is not enough), they should be in
> normal comments /* ... */ so that they aren't in docparse.
> 
> IMHO docparse can mention some commit if wanted to add some description,
> but just as 5f955f26f3d4 or 5f955f26f3d4 ("xfs: report crtime and
> attribute flags to statx") if the commit subject is a description
> itself.
> 
> http://linux-test-project.github.io/metadata/metadata.stable.html#statx04
> 
> Kind regards,
> Petr
> 
>   testcases/kernel/syscalls/statx/statx04.c | 35 +----------------------
>   1 file changed, 1 insertion(+), 34 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/statx/statx04.c b/testcases/kernel/syscalls/statx/statx04.c
> index 98f9a6315..6c562b3d7 100644
> --- a/testcases/kernel/syscalls/statx/statx04.c
> +++ b/testcases/kernel/syscalls/statx/statx04.c
> @@ -14,41 +14,8 @@
>    * - STATX_ATTR_NODUMP: File is not a candidate for backup when a backup
>    *                        program such as dump(8) is run.
>    *
> - * xfs filesystem doesn't support STATX_ATTR_COMPRESSED flag, so we only test
> + * XFS filesystem doesn't support STATX_ATTR_COMPRESSED flag, so we only test
>    * three other flags.
> - *
> - * ext2, ext4, btrfs, xfs and tmpfs support statx syscall since the following commit
> - *
> - *  commit 93bc420ed41df63a18ae794101f7cbf45226a6ef
> - *  Author: yangerkun <yangerkun@huawei.com>
> - *  Date:   Mon Feb 18 09:07:02 2019 +0800
> - *
> - *  ext2: support statx syscall
> - *
> - *  commit 99652ea56a4186bc5bf8a3721c5353f41b35ebcb
> - *  Author: David Howells <dhowells@redhat.com>
> - *  Date:   Fri Mar 31 18:31:56 2017 +0100
> - *
> - *  ext4: Add statx support
> - *
> - *  commit 04a87e3472828f769a93655d7c64a27573bdbc2c
> - *  Author: Yonghong Song <yhs@fb.com>
> - *  Date:   Fri May 12 15:07:43 2017 -0700
> - *
> - *  Btrfs: add statx support
> - *
> - *  commit 5f955f26f3d42d04aba65590a32eb70eedb7f37d
> - *  Author: Darrick J. Wong <darrick.wong@oracle.com>
> - *  Date:   Fri Mar 31 18:32:03 2017 +0100
> - *
> - *  xfs: report crtime and attribute flags to statx
> - *
> - *  commit e408e695f5f1f60d784913afc45ff2c387a5aeb8
> - *  Author: Theodore Ts'o <tytso@mit.edu>
> - *  Date:   Thu Jul 14 21:59:12 2022 -0400
> - *
> - *  mm/shmem: support FS_IOC_[SG]ETFLAGS in tmpfs
> - *
>    */
>   
>   #define _GNU_SOURCE

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] statx04: Remove kernel comments in docparse

[xuyang2018.jy]

Hi Petr

On my owner metadata.html, I don't see  commit title(I guess network 
problem)


Tag	Info
linux-git  93bc420ed41d

linux-git 99652ea56a41

linux-git  04a87e347282

linux-git 5f955f26f3d4

linux-git e408e695f5f1

So for me, If remove kernel commit in description, then I only see
useless commit id number, unless I go to search them in kernel org.

I prefer to keep these kernel commit comment.

Best Regards
Yang Xu

> 
> Hi Petr
> 
> Oh, I see commit title in metadata that I don't see it before.
> 
> Tag	   Info
> linux-git 93bc420ed41d ("ext2: support statx syscall")
> 
> linux-git 99652ea56a41 ("ext4: Add statx support")
> 
> linux-git 04a87e347282 ("Btrfs: add statx support")
> 
> linux-git 5f955f26f3d4 ("xfs: report crtime and attribute flags to statx")
> 
> 
> But user that doesn't use metadata(miss this usage or miss dependent
> package ie rubygem-asciidoctor), then they only see some numbers in tag
> but know nothing.
> 
> IMO, it is not clear like min_kever.
> 
> Best Regards
> Yang Xu
>> They are defined in .tags, having in docparse results into poor
>> formating in metadata.{html,pdf}.
>>
>> Signed-off-by: Petr Vorel <pvorel@suse.cz>
>> ---
>> Hi Li,
>>
>> I've done cleanup like this in the past, but sending a patch just to
>> make consensus about it. If we prefer to have git commits like this in
>> the code (i.e. if being in .tags is not enough), they should be in
>> normal comments /* ... */ so that they aren't in docparse.
>>
>> IMHO docparse can mention some commit if wanted to add some description,
>> but just as 5f955f26f3d4 or 5f955f26f3d4 ("xfs: report crtime and
>> attribute flags to statx") if the commit subject is a description
>> itself.
>>
>> http://linux-test-project.github.io/metadata/metadata.stable.html#statx04
>>
>> Kind regards,
>> Petr
>>
>>    testcases/kernel/syscalls/statx/statx04.c | 35 +----------------------
>>    1 file changed, 1 insertion(+), 34 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/statx/statx04.c b/testcases/kernel/syscalls/statx/statx04.c
>> index 98f9a6315..6c562b3d7 100644
>> --- a/testcases/kernel/syscalls/statx/statx04.c
>> +++ b/testcases/kernel/syscalls/statx/statx04.c
>> @@ -14,41 +14,8 @@
>>     * - STATX_ATTR_NODUMP: File is not a candidate for backup when a backup
>>     *                        program such as dump(8) is run.
>>     *
>> - * xfs filesystem doesn't support STATX_ATTR_COMPRESSED flag, so we only test
>> + * XFS filesystem doesn't support STATX_ATTR_COMPRESSED flag, so we only test
>>     * three other flags.
>> - *
>> - * ext2, ext4, btrfs, xfs and tmpfs support statx syscall since the following commit
>> - *
>> - *  commit 93bc420ed41df63a18ae794101f7cbf45226a6ef
>> - *  Author: yangerkun <yangerkun@huawei.com>
>> - *  Date:   Mon Feb 18 09:07:02 2019 +0800
>> - *
>> - *  ext2: support statx syscall
>> - *
>> - *  commit 99652ea56a4186bc5bf8a3721c5353f41b35ebcb
>> - *  Author: David Howells <dhowells@redhat.com>
>> - *  Date:   Fri Mar 31 18:31:56 2017 +0100
>> - *
>> - *  ext4: Add statx support
>> - *
>> - *  commit 04a87e3472828f769a93655d7c64a27573bdbc2c
>> - *  Author: Yonghong Song <yhs@fb.com>
>> - *  Date:   Fri May 12 15:07:43 2017 -0700
>> - *
>> - *  Btrfs: add statx support
>> - *
>> - *  commit 5f955f26f3d42d04aba65590a32eb70eedb7f37d
>> - *  Author: Darrick J. Wong <darrick.wong@oracle.com>
>> - *  Date:   Fri Mar 31 18:32:03 2017 +0100
>> - *
>> - *  xfs: report crtime and attribute flags to statx
>> - *
>> - *  commit e408e695f5f1f60d784913afc45ff2c387a5aeb8
>> - *  Author: Theodore Ts'o <tytso@mit.edu>
>> - *  Date:   Thu Jul 14 21:59:12 2022 -0400
>> - *
>> - *  mm/shmem: support FS_IOC_[SG]ETFLAGS in tmpfs
>> - *
>>     */
>>    
>>    #define _GNU_SOURCE
> 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] statx04: Remove kernel comments in docparse

[Petr Vorel]

> Hi Petr

> On my owner metadata.html, I don't see  commit title(I guess network 
> problem)

No, you need to set environment variable LINUX_GIT, LINUX_STABLE_GIT and
GLIBC_GIT. There is also warning:

$ unset LINUX_GIT LINUX_STABLE_GIT GLIBC_GIT
$ cd metadata && make
WARN: git repository linux-git not defined. Define it in $LINUX_GIT
WARN: git repository linux-stable-git not defined. Define it in $LINUX_STABLE_GIT
WARN: git repository glibc-git not defined. Define it in $GLIBC_GIT


> Tag	Info
> linux-git  93bc420ed41d

> linux-git 99652ea56a41

> linux-git  04a87e347282

> linux-git 5f955f26f3d4

> linux-git e408e695f5f1

> So for me, If remove kernel commit in description, then I only see
> useless commit id number, unless I go to search them in kernel org.

> I prefer to keep these kernel commit comment.

I would still suggest to have change them as normal C comments - that's enough
for reading C source. We already added released docs to
linux-test-project.github.io [1], I plan to add hook to to push master version
there after each commit and stable after release (after pushing tag).
Then we can add this link to README.md to propagate docs.

Kind regards,
Petr

[1] http://linux-test-project.github.io/metadata/metadata.stable.html

> Best Regards
> Yang Xu


> > Hi Petr

> > Oh, I see commit title in metadata that I don't see it before.

> > Tag	   Info
> > linux-git 93bc420ed41d ("ext2: support statx syscall")

> > linux-git 99652ea56a41 ("ext4: Add statx support")

> > linux-git 04a87e347282 ("Btrfs: add statx support")

> > linux-git 5f955f26f3d4 ("xfs: report crtime and attribute flags to statx")


> > But user that doesn't use metadata(miss this usage or miss dependent
> > package ie rubygem-asciidoctor), then they only see some numbers in tag
> > but know nothing.

> > IMO, it is not clear like min_kever.

> > Best Regards
> > Yang Xu
> >> They are defined in .tags, having in docparse results into poor
> >> formating in metadata.{html,pdf}.

> >> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> >> ---
> >> Hi Li,

> >> I've done cleanup like this in the past, but sending a patch just to
> >> make consensus about it. If we prefer to have git commits like this in
> >> the code (i.e. if being in .tags is not enough), they should be in
> >> normal comments /* ... */ so that they aren't in docparse.

> >> IMHO docparse can mention some commit if wanted to add some description,
> >> but just as 5f955f26f3d4 or 5f955f26f3d4 ("xfs: report crtime and
> >> attribute flags to statx") if the commit subject is a description
> >> itself.

> >> http://linux-test-project.github.io/metadata/metadata.stable.html#statx04

> >> Kind regards,
> >> Petr

> >>    testcases/kernel/syscalls/statx/statx04.c | 35 +----------------------
> >>    1 file changed, 1 insertion(+), 34 deletions(-)

> >> diff --git a/testcases/kernel/syscalls/statx/statx04.c b/testcases/kernel/syscalls/statx/statx04.c
> >> index 98f9a6315..6c562b3d7 100644
> >> --- a/testcases/kernel/syscalls/statx/statx04.c
> >> +++ b/testcases/kernel/syscalls/statx/statx04.c
> >> @@ -14,41 +14,8 @@
> >>     * - STATX_ATTR_NODUMP: File is not a candidate for backup when a backup
> >>     *                        program such as dump(8) is run.
> >>     *
> >> - * xfs filesystem doesn't support STATX_ATTR_COMPRESSED flag, so we only test
> >> + * XFS filesystem doesn't support STATX_ATTR_COMPRESSED flag, so we only test
> >>     * three other flags.
> >> - *
> >> - * ext2, ext4, btrfs, xfs and tmpfs support statx syscall since the following commit
> >> - *
> >> - *  commit 93bc420ed41df63a18ae794101f7cbf45226a6ef
> >> - *  Author: yangerkun <yangerkun@huawei.com>
> >> - *  Date:   Mon Feb 18 09:07:02 2019 +0800
> >> - *
> >> - *  ext2: support statx syscall
> >> - *
> >> - *  commit 99652ea56a4186bc5bf8a3721c5353f41b35ebcb
> >> - *  Author: David Howells <dhowells@redhat.com>
> >> - *  Date:   Fri Mar 31 18:31:56 2017 +0100
> >> - *
> >> - *  ext4: Add statx support
> >> - *
> >> - *  commit 04a87e3472828f769a93655d7c64a27573bdbc2c
> >> - *  Author: Yonghong Song <yhs@fb.com>
> >> - *  Date:   Fri May 12 15:07:43 2017 -0700
> >> - *
> >> - *  Btrfs: add statx support
> >> - *
> >> - *  commit 5f955f26f3d42d04aba65590a32eb70eedb7f37d
> >> - *  Author: Darrick J. Wong <darrick.wong@oracle.com>
> >> - *  Date:   Fri Mar 31 18:32:03 2017 +0100
> >> - *
> >> - *  xfs: report crtime and attribute flags to statx
> >> - *
> >> - *  commit e408e695f5f1f60d784913afc45ff2c387a5aeb8
> >> - *  Author: Theodore Ts'o <tytso@mit.edu>
> >> - *  Date:   Thu Jul 14 21:59:12 2022 -0400
> >> - *
> >> - *  mm/shmem: support FS_IOC_[SG]ETFLAGS in tmpfs
> >> - *
> >>     */

> >>    #define _GNU_SOURCE


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] statx04: Remove kernel comments in docparse

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1021 bytes --]

Hi Petr,

On Fri, Aug 19, 2022 at 5:07 PM Petr Vorel <pvorel@suse.cz> wrote:

> They are defined in .tags, having in docparse results into poor
> formating in metadata.{html,pdf}.
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Hi Li,
>
> I've done cleanup like this in the past, but sending a patch just to
> make consensus about it. If we prefer to have git commits like this in
> the code (i.e. if being in .tags is not enough), they should be in
> normal comments /* ... */ so that they aren't in docparse.


> IMHO docparse can mention some commit if wanted to add some description,
> but just as 5f955f26f3d4 or 5f955f26f3d4 ("xfs: report crtime and
> attribute flags to statx") if the commit subject is a description
> itself.
>

Good to know this. I agree to remove those commit info as you
did in this patch. To be honest, I don't think that helps people in
code reading, they are just as reductant info pointer to the feature's
commit id.

Reviewed-by: Li Wang <liwang@redhat.com>


-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 2162 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] statx04: Remove kernel comments in docparse

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 821 bytes --]

Hi Xu, Petr,

On Fri, Aug 19, 2022 at 7:23 PM Petr Vorel <pvorel@suse.cz> wrote:


> > So for me, If remove kernel commit in description, then I only see
> > useless commit id number, unless I go to search them in kernel org.
>
> > I prefer to keep these kernel commit comment.
>

If so, adding normal C comments should be a workaround
(as Petr proposed), but I doubt how much value for reserving that:).


>
> I would still suggest to have change them as normal C comments - that's
> enough
> for reading C source. We already added released docs to
> linux-test-project.github.io [1], I plan to add hook to to push master
> version
> there after each commit and stable after release (after pushing tag).
> Then we can add this link to README.md to propagate docs.
>

+1, sounds good to me, thanks!

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 1779 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] statx04: Remove kernel comments in docparse

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 728 bytes --]

> So for me, If remove kernel commit in description, then I only see
> useless commit id number, unless I go to search them in kernel org.
>

Not exactly, another thing to make people's attention is test fail with
hint,
then we can simply click the link.

    58	HINT: You _MAY_ be missing kernel fixes:
    59	
    60	https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93bc420ed41d
    61	https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99652ea56a41
    62	https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=04a87e347282
    63  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f955f26f3d4


-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 1921 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] statx04: Remove kernel comments in docparse

[xuyang2018.jy]

Hi Li

>     So for me, If remove kernel commit in description, then I only see
>     useless commit id number, unless I go to search them in kernel org.
> 
> 
> Not exactly, another thing to make people's attention is test fail with 
> hint,
> then we can simply click the link.

I remember Amir also added a similar feature[1] to xfstests when he 
become xfs 5.10 stable kernel maintainer.

So maybe we can add a third colume in tst_test struct's tag field  to 
cover kernel commit title? Then this kernel comment can be removed.

Also, I don't have strong obejection to this patch because it seems 
duplicate, but I just want to keep kernel commit title by using simple way.

[1]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=4fb72cffd9e3251149d9fa25daa96b2e26b62eab

Best Regards
Yang Xu
> 

>      58	HINT: You _MAY_ be missing kernel fixes:
>      59	
>      60	https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93bc420ed41d  <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93bc420ed41d>
>      61	https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99652ea56a41  <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99652ea56a41>
>      62	https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=04a87e347282  <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=04a87e347282>  
>      63https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f955f26f3d4  <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f955f26f3d4>
> 
> 
> -- 
> Regards,
> Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] statx04: Remove kernel comments in docparse

[Petr Vorel]

Hi all,

thanks all for their input.

> Hi Li

> >     So for me, If remove kernel commit in description, then I only see
> >     useless commit id number, unless I go to search them in kernel org.


> > Not exactly, another thing to make people's attention is test fail with 
> > hint,
> > then we can simply click the link.

> I remember Amir also added a similar feature[1] to xfstests when he 
> become xfs 5.10 stable kernel maintainer.

> So maybe we can add a third colume in tst_test struct's tag field  to 
> cover kernel commit title? Then this kernel comment can be removed.

Amir got inspiration from LTP, but I'm not a big fan of adding manually git
subject which is now added by script.  Also I'd like to address the change we
agree also in fanotify sources.

> Also, I don't have strong obejection to this patch because it seems 
> duplicate, but I just want to keep kernel commit title by using simple way.

I also wonder why people add commits in this verbose form:

 *  commit 93bc420ed41df63a18ae794101f7cbf45226a6ef
 *  Author: yangerkun <yangerkun@huawei.com>
 *  Date:   Mon Feb 18 09:07:02 2019 +0800
 *
 *  ext2: support statx syscall

If commits are really necessary I'd add them inline, i.e.:

$ git log --pretty=format:'%h ("%s")' -1 93bc420ed41d
93bc420ed41d ("ext2: support statx syscall")

But again, having git hash at the bottom and then full commit on the top looks
to me just a redundancy (I often want to see changes in kernel thus run git show
hash anyway + there is that link in docparse.

So let's wait for opinion of other people.

Kind regards,
Petr

> [1]https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=4fb72cffd9e3251149d9fa25daa96b2e26b62eab

> Best Regards
> Yang Xu


> >      58	HINT: You _MAY_ be missing kernel fixes:
> >      59	
> >      60	https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93bc420ed41d  <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93bc420ed41d>
> >      61	https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99652ea56a41  <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99652ea56a41>
> >      62	https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=04a87e347282  <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=04a87e347282>  
> >      63https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f955f26f3d4  <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5f955f26f3d4>


> > -- 
> > Regards,
> > Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] statx04: Remove kernel comments in docparse

[Li Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1696 bytes --]

Hi Xu, Petr,

Petr Vorel <pvorel@suse.cz> wrote:

> I remember Amir also added a similar feature[1] to xfstests when he
> > become xfs 5.10 stable kernel maintainer.
>
> > So maybe we can add a third colume in tst_test struct's tag field  to
> > cover kernel commit title? Then this kernel comment can be removed.
>

We can achieve that but the complexity outweighs the benefits (for LTP).


>
> Amir got inspiration from LTP, but I'm not a big fan of adding manually git
> subject which is now added by script.  Also I'd like to address the change
> we
> agree also in fanotify sources.


> > Also, I don't have strong obejection to this patch because it seems
> > duplicate, but I just want to keep kernel commit title by using simple
> way.
>
> I also wonder why people add commits in this verbose form:
>

That's legacy way in writing comments, but obviously it need unify in format
for better management in future.



>
>  *  commit 93bc420ed41df63a18ae794101f7cbf45226a6ef
>  *  Author: yangerkun <yangerkun@huawei.com>
>  *  Date:   Mon Feb 18 09:07:02 2019 +0800
>  *
>  *  ext2: support statx syscall
>
> If commits are really necessary I'd add them inline, i.e.:
>
> $ git log --pretty=format:'%h ("%s")' -1 93bc420ed41d
> 93bc420ed41d ("ext2: support statx syscall")
>

+1, at most we add this format in normal C comments.
https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/fanotify/fanotify14.c#L17



> But again, having git hash at the bottom and then full commit on the top
> looks
> to me just a redundancy (I often want to see changes in kernel thus run
> git show
> hash anyway + there is that link in docparse.
>

me as well.

-- 
Regards,
Li Wang

[-- Attachment #1.2: Type: text/html, Size: 3582 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] statx04: Remove kernel comments in docparse

[Petr Vorel]

Hi Li, Xu, all,

> Hi Xu, Petr,

> Petr Vorel <pvorel@suse.cz> wrote:

> > I remember Amir also added a similar feature[1] to xfstests when he
> > > become xfs 5.10 stable kernel maintainer.

> > > So maybe we can add a third colume in tst_test struct's tag field  to
> > > cover kernel commit title? Then this kernel comment can be removed.


> We can achieve that but the complexity outweighs the benefits (for LTP).
+1



> > Amir got inspiration from LTP, but I'm not a big fan of adding manually git
> > subject which is now added by script.  Also I'd like to address the change
> > we
> > agree also in fanotify sources.


> > > Also, I don't have strong obejection to this patch because it seems
> > > duplicate, but I just want to keep kernel commit title by using simple
> > way.

> > I also wonder why people add commits in this verbose form:


> That's legacy way in writing comments, but obviously it need unify in format
> for better management in future.
+1 (that was the reason, why I posted it even it's trivial formatting patch, I
should also have posted it as RFC).



> >  *  commit 93bc420ed41df63a18ae794101f7cbf45226a6ef
> >  *  Author: yangerkun <yangerkun@huawei.com>
> >  *  Date:   Mon Feb 18 09:07:02 2019 +0800
> >  *
> >  *  ext2: support statx syscall

> > If commits are really necessary I'd add them inline, i.e.:

> > $ git log --pretty=format:'%h ("%s")' -1 93bc420ed41d
> > 93bc420ed41d ("ext2: support statx syscall")


> +1, at most we add this format in normal C comments.
> https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/fanotify/fanotify14.c#L17



> > But again, having git hash at the bottom and then full commit on the top
> > looks
> > to me just a redundancy (I often want to see changes in kernel thus run
> > git show
> > hash anyway + there is that link in docparse.


> me as well.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp