All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] travis: add checkpatch analysis
@ 2019-03-19 14:28 roman.stratiienko
  2019-03-19 14:45 ` Jan Kiszka
  0 siblings, 1 reply; 6+ messages in thread
From: roman.stratiienko @ 2019-03-19 14:28 UTC (permalink / raw)
  To: xenomai; +Cc: Roman Stratiienko

From: Roman Stratiienko <roman.stratiienko@globallogic.com>

Check last 10 commits with checkpatch.pl before build

Signed-off-by: Roman Stratiienko <roman.stratiienko@globallogic.com>
---
 .travis.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.travis.yml b/.travis.yml
index 0cd9e571f..6fb604bf2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -78,6 +78,7 @@ before_script:
   - popd
 
 script:
+  - ${KDIR}/scripts/checkpatch.pl --git HEAD~10..HEAD
   - scripts/prepare-kernel.sh --ipipe=/tmp/ipipe.patch --arch=${ARCH} --linux=${KDIR}
   - pushd ${KDIR}
   - make -j $(nproc) olddefconfig
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] travis: add checkpatch analysis
  2019-03-19 14:28 [PATCH] travis: add checkpatch analysis roman.stratiienko
@ 2019-03-19 14:45 ` Jan Kiszka
  2019-03-19 14:59   ` Roman Stratiienko
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2019-03-19 14:45 UTC (permalink / raw)
  To: roman.stratiienko, xenomai

On 19.03.19 15:28, roman.stratiienko--- via Xenomai wrote:
> From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> 
> Check last 10 commits with checkpatch.pl before build
> 
> Signed-off-by: Roman Stratiienko <roman.stratiienko@globallogic.com>
> ---
>   .travis.yml | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 0cd9e571f..6fb604bf2 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -78,6 +78,7 @@ before_script:
>     - popd
>   
>   script:
> +  - ${KDIR}/scripts/checkpatch.pl --git HEAD~10..HEAD
>     - scripts/prepare-kernel.sh --ipipe=/tmp/ipipe.patch --arch=${ARCH} --linux=${KDIR}
>     - pushd ${KDIR}
>     - make -j $(nproc) olddefconfig
> 

Hmm, not convinced about enforced checking. That will fail the build, even if 
there might be a legitimate reason for some exception, no?

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] travis: add checkpatch analysis
  2019-03-19 14:45 ` Jan Kiszka
@ 2019-03-19 14:59   ` Roman Stratiienko
  2019-03-19 18:15     ` Jan Kiszka
  0 siblings, 1 reply; 6+ messages in thread
From: Roman Stratiienko @ 2019-03-19 14:59 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: xenomai

Currently I see a lot of checkpatch warnings, that can not be fixed in
xenomai project.
But checkpatch errors usually could be fixed, so we should try I think.


On Tue, Mar 19, 2019 at 4:45 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:

> On 19.03.19 15:28, roman.stratiienko--- via Xenomai wrote:
> > From: Roman Stratiienko <roman.stratiienko@globallogic.com>
> >
> > Check last 10 commits with checkpatch.pl before build
> >
> > Signed-off-by: Roman Stratiienko <roman.stratiienko@globallogic.com>
> > ---
> >   .travis.yml | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/.travis.yml b/.travis.yml
> > index 0cd9e571f..6fb604bf2 100644
> > --- a/.travis.yml
> > +++ b/.travis.yml
> > @@ -78,6 +78,7 @@ before_script:
> >     - popd
> >
> >   script:
> > +  - ${KDIR}/scripts/checkpatch.pl --git HEAD~10..HEAD
> >     - scripts/prepare-kernel.sh --ipipe=/tmp/ipipe.patch --arch=${ARCH}
> --linux=${KDIR}
> >     - pushd ${KDIR}
> >     - make -j $(nproc) olddefconfig
> >
>
> Hmm, not convinced about enforced checking. That will fail the build, even
> if
> there might be a legitimate reason for some exception, no?
>






>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> Corporate Competence Center Embedded Linux
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] travis: add checkpatch analysis
  2019-03-19 14:59   ` Roman Stratiienko
@ 2019-03-19 18:15     ` Jan Kiszka
  2019-03-22 19:45       ` Roman Stratiienko
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2019-03-19 18:15 UTC (permalink / raw)
  To: Roman Stratiienko; +Cc: xenomai

On 19.03.19 15:59, Roman Stratiienko wrote:
> Currently I see a lot of checkpatch warnings, that can not be fixed in xenomai 
> project.
> But checkpatch errors usually could be fixed, so we should try I think.
> 

I don't disagree on the general goal of clean coding style. I just don't want to 
open too many fronts at the same time. If the checker nags us with reports that 
no one reads in the end, that will not help. And we know that there corners like 
rtnet which needs more than some smaller tweaks (and will currently only 
generate errors unless you break the pre-existing style).

If we should agree on using automated style checking, I would then redefine the 
range to master..next, because that is what we are currently adding.

Jan

> 
> On Tue, Mar 19, 2019 at 4:45 PM Jan Kiszka <jan.kiszka@siemens.com 
> <mailto:jan.kiszka@siemens.com>> wrote:
> 
>     On 19.03.19 15:28, roman.stratiienko--- via Xenomai wrote:
>      > From: Roman Stratiienko <roman.stratiienko@globallogic.com
>     <mailto:roman.stratiienko@globallogic.com>>
>      >
>      > Check last 10 commits with checkpatch.pl <http://checkpatch.pl> before build
>      >
>      > Signed-off-by: Roman Stratiienko <roman.stratiienko@globallogic.com
>     <mailto:roman.stratiienko@globallogic.com>>
>      > ---
>      >   .travis.yml | 1 +
>      >   1 file changed, 1 insertion(+)
>      >
>      > diff --git a/.travis.yml b/.travis.yml
>      > index 0cd9e571f..6fb604bf2 100644
>      > --- a/.travis.yml
>      > +++ b/.travis.yml
>      > @@ -78,6 +78,7 @@ before_script:
>      >     - popd
>      >
>      >   script:
>      > +  - ${KDIR}/scripts/checkpatch.pl <http://checkpatch.pl> --git HEAD~10..HEAD
>      >     - scripts/prepare-kernel.sh --ipipe=/tmp/ipipe.patch --arch=${ARCH}
>     --linux=${KDIR}
>      >     - pushd ${KDIR}
>      >     - make -j $(nproc) olddefconfig
>      >
> 
>     Hmm, not convinced about enforced checking. That will fail the build, even if
>     there might be a legitimate reason for some exception, no?
> 
> 
> 
> 
> 
> 
>     Jan
> 
>     -- 
>     Siemens AG, Corporate Technology, CT RDA IOT SES-DE
>     Corporate Competence Center Embedded Linux
> 
> 
> 

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] travis: add checkpatch analysis
  2019-03-19 18:15     ` Jan Kiszka
@ 2019-03-22 19:45       ` Roman Stratiienko
  2019-03-22 20:54         ` Jan Kiszka
  0 siblings, 1 reply; 6+ messages in thread
From: Roman Stratiienko @ 2019-03-22 19:45 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: xenomai

Thank you for feedback. After testing of this patch I faced some challenges:

1. checkpatch.pl from kernel tree always different. Old kernel checkpatch
doesn't support git option. So we need to find a way to use ommon utility
for all kernel or just enable checkpatch only on most recent kernel
2. Even if there is no checkpatch errors, but warnings still present, exit
code will be false. And I can't find command line option to override it.
3. Getting delta commits in master..next range isn't a trivial task. Travis
clone only single branch with - - depth=50 option. Unshallowing entire
repository will take some amount of time, that will slow down build process
and it is not what me want from CI system.



вт, 19 мар. 2019 г., 20:15 Jan Kiszka <jan.kiszka@siemens.com>:

> On 19.03.19 15:59, Roman Stratiienko wrote:
> > Currently I see a lot of checkpatch warnings, that can not be fixed in
> xenomai
> > project.
> > But checkpatch errors usually could be fixed, so we should try I think.
> >
>
> I don't disagree on the general goal of clean coding style. I just don't
> want to
> open too many fronts at the same time. If the checker nags us with reports
> that
> no one reads in the end, that will not help. And we know that there
> corners like
> rtnet which needs more than some smaller tweaks (and will currently only
> generate errors unless you break the pre-existing style).
>
> If we should agree on using automated style checking, I would then
> redefine the
> range to master..next, because that is what we are currently adding.
>
> Jan
>
> >
> > On Tue, Mar 19, 2019 at 4:45 PM Jan Kiszka <jan.kiszka@siemens.com
> > <mailto:jan.kiszka@siemens.com>> wrote:
> >
> >     On 19.03.19 15:28, roman.stratiienko--- via Xenomai wrote:
> >      > From: Roman Stratiienko <roman.stratiienko@globallogic.com
> >     <mailto:roman.stratiienko@globallogic.com>>
> >      >
> >      > Check last 10 commits with checkpatch.pl <http://checkpatch.pl>
> before build
> >      >
> >      > Signed-off-by: Roman Stratiienko <
> roman.stratiienko@globallogic.com
> >     <mailto:roman.stratiienko@globallogic.com>>
> >      > ---
> >      >   .travis.yml | 1 +
> >      >   1 file changed, 1 insertion(+)
> >      >
> >      > diff --git a/.travis.yml b/.travis.yml
> >      > index 0cd9e571f..6fb604bf2 100644
> >      > --- a/.travis.yml
> >      > +++ b/.travis.yml
> >      > @@ -78,6 +78,7 @@ before_script:
> >      >     - popd
> >      >
> >      >   script:
> >      > +  - ${KDIR}/scripts/checkpatch.pl <http://checkpatch.pl> --git
> HEAD~10..HEAD
> >      >     - scripts/prepare-kernel.sh --ipipe=/tmp/ipipe.patch
> --arch=${ARCH}
> >     --linux=${KDIR}
> >      >     - pushd ${KDIR}
> >      >     - make -j $(nproc) olddefconfig
> >      >
> >
> >     Hmm, not convinced about enforced checking. That will fail the
> build, even if
> >     there might be a legitimate reason for some exception, no?
> >
> >
> >
> >
> >
> >
> >     Jan
> >
> >     --
> >     Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> >     Corporate Competence Center Embedded Linux
> >
> >
> >
>
> --
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> Corporate Competence Center Embedded Linux
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] travis: add checkpatch analysis
  2019-03-22 19:45       ` Roman Stratiienko
@ 2019-03-22 20:54         ` Jan Kiszka
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2019-03-22 20:54 UTC (permalink / raw)
  To: Roman Stratiienko; +Cc: xenomai

On 22.03.19 20:45, Roman Stratiienko wrote:
> Thank you for feedback. After testing of this patch I faced some challenges:
> 
> 1. checkpatch.pl <http://checkpatch.pl> from kernel tree always different. Old 
> kernel checkpatch doesn't support git option. So we need to find a way to use 
> ommon utility for all kernel or just enable checkpatch only on most recent kernel
> 2. Even if there is no checkpatch errors, but warnings still present, exit code 
> will be false. And I can't find command line option to override it.
> 3. Getting delta commits in master..next range isn't a trivial task. Travis 
> clone only single branch with - - depth=50 option. Unshallowing entire 
> repository will take some amount of time, that will slow down build process and 
> it is not what me want from CI system.
> 

Yeah... But I guess I start to could run it locally.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-22 20:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-19 14:28 [PATCH] travis: add checkpatch analysis roman.stratiienko
2019-03-19 14:45 ` Jan Kiszka
2019-03-19 14:59   ` Roman Stratiienko
2019-03-19 18:15     ` Jan Kiszka
2019-03-22 19:45       ` Roman Stratiienko
2019-03-22 20:54         ` Jan Kiszka

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.