| Message ID | 1426724244-9611-1-git-send-email-mike.holmes@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
ping On 18 March 2015 at 20:17, Mike Holmes <mike.holmes@linaro.org> wrote: > ODP has not adopted a style that can be universally applied with a tool > such as astyle. > Remove astyle leaving only the cleanup for whitespace and checkpatch > elements for checking src files before a patch is created. > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > scripts/odp_check | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/scripts/odp_check b/scripts/odp_check > index 09c859b..33809dc 100755 > --- a/scripts/odp_check > +++ b/scripts/odp_check > @@ -1,8 +1,6 @@ > #!/bin/bash > # > -# This script is an indenter, white space remover, > -# formatter, and beautifier and general source file > -# clean up for the ODP project. > +# This script is a clean up for the ODP project src files. > # > # Usage > # ./scripts/opd_check <path/filename> > @@ -10,11 +8,5 @@ set -e > > DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" > > -if ! type "astyle" >/dev/null >/dev/null; then > - echo "Please install astyle from http://astyle.sourceforge.net/" > - exit -1 > -fi > - > -astyle --style=linux --indent=force-tab=8 --align-pointer=name $1 > $DIR/cleanfile $1 > $DIR/checkpatch.pl -f $1 > -- > 2.1.0 > >
General comment: Not clear whether this is checking (sort of implied by the name odp_check) or changing files to match a predefined style. If the latter then some other name should be used as I wouldn't expect a "check" function to write anything except diagnostics. Some clarity on intended use would help here. On Wed, Mar 18, 2015 at 7:17 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > ODP has not adopted a style that can be universally applied with a tool > such as astyle. > Remove astyle leaving only the cleanup for whitespace and checkpatch > elements for checking src files before a patch is created. > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > scripts/odp_check | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/scripts/odp_check b/scripts/odp_check > index 09c859b..33809dc 100755 > --- a/scripts/odp_check > +++ b/scripts/odp_check > @@ -1,8 +1,6 @@ > #!/bin/bash > # > -# This script is an indenter, white space remover, > -# formatter, and beautifier and general source file > -# clean up for the ODP project. > +# This script is a clean up for the ODP project src files. > # > # Usage > # ./scripts/opd_check <path/filename> > Typo: opd_check instead of odp_check > @@ -10,11 +8,5 @@ set -e > > DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" > > -if ! type "astyle" >/dev/null >/dev/null; then > - echo "Please install astyle from http://astyle.sourceforge.net/" > - exit -1 > -fi > - > -astyle --style=linux --indent=force-tab=8 --align-pointer=name $1 > $DIR/cleanfile $1 > $DIR/checkpatch.pl -f $1 > -- > 2.1.0 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 24 March 2015 at 18:20, Bill Fischofer <bill.fischofer@linaro.org> wrote: > General comment: Not clear whether this is checking (sort of implied by > the name odp_check) or changing files to match a predefined style. If the > latter then some other name should be used as I wouldn't expect a "check" > function to write anything except diagnostics. Some clarity on intended > use would help here. > It used to reformat fully - be we have not adopted that, so after this patch it just fixes whitespace and does checkpatch equivalent but on a given file rather than a patch. > > On Wed, Mar 18, 2015 at 7:17 PM, Mike Holmes <mike.holmes@linaro.org> > wrote: > >> ODP has not adopted a style that can be universally applied with a tool >> such as astyle. >> Remove astyle leaving only the cleanup for whitespace and checkpatch >> elements for checking src files before a patch is created. >> >> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >> --- >> scripts/odp_check | 10 +--------- >> 1 file changed, 1 insertion(+), 9 deletions(-) >> >> diff --git a/scripts/odp_check b/scripts/odp_check >> index 09c859b..33809dc 100755 >> --- a/scripts/odp_check >> +++ b/scripts/odp_check >> @@ -1,8 +1,6 @@ >> #!/bin/bash >> # >> -# This script is an indenter, white space remover, >> -# formatter, and beautifier and general source file >> -# clean up for the ODP project. >> +# This script is a clean up for the ODP project src files. >> # >> # Usage >> # ./scripts/opd_check <path/filename> >> > > Typo: opd_check instead of odp_check > > >> @@ -10,11 +8,5 @@ set -e >> >> DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" >> >> -if ! type "astyle" >/dev/null >/dev/null; then >> - echo "Please install astyle from http://astyle.sourceforge.net/" >> - exit -1 >> -fi >> - >> -astyle --style=linux --indent=force-tab=8 --align-pointer=name $1 >> $DIR/cleanfile $1 >> $DIR/checkpatch.pl -f $1 >> -- >> 2.1.0 >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > >
If it's changing files, it shouldn't be (misleadingly) named "check", which implies that it's just examining them. odp_fixwhitespace perhaps? On Wed, Mar 25, 2015 at 6:29 AM, Mike Holmes <mike.holmes@linaro.org> wrote: > > > On 24 March 2015 at 18:20, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> General comment: Not clear whether this is checking (sort of implied by >> the name odp_check) or changing files to match a predefined style. If the >> latter then some other name should be used as I wouldn't expect a "check" >> function to write anything except diagnostics. Some clarity on intended >> use would help here. >> > > It used to reformat fully - be we have not adopted that, so after this > patch it just fixes whitespace and does checkpatch equivalent but on a > given file rather than a patch. > > >> >> On Wed, Mar 18, 2015 at 7:17 PM, Mike Holmes <mike.holmes@linaro.org> >> wrote: >> >>> ODP has not adopted a style that can be universally applied with a tool >>> such as astyle. >>> Remove astyle leaving only the cleanup for whitespace and checkpatch >>> elements for checking src files before a patch is created. >>> >>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >>> --- >>> scripts/odp_check | 10 +--------- >>> 1 file changed, 1 insertion(+), 9 deletions(-) >>> >>> diff --git a/scripts/odp_check b/scripts/odp_check >>> index 09c859b..33809dc 100755 >>> --- a/scripts/odp_check >>> +++ b/scripts/odp_check >>> @@ -1,8 +1,6 @@ >>> #!/bin/bash >>> # >>> -# This script is an indenter, white space remover, >>> -# formatter, and beautifier and general source file >>> -# clean up for the ODP project. >>> +# This script is a clean up for the ODP project src files. >>> # >>> # Usage >>> # ./scripts/opd_check <path/filename> >>> >> >> Typo: opd_check instead of odp_check >> >> >>> @@ -10,11 +8,5 @@ set -e >>> >>> DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" >>> >>> -if ! type "astyle" >/dev/null >/dev/null; then >>> - echo "Please install astyle from http://astyle.sourceforge.net/" >>> - exit -1 >>> -fi >>> - >>> -astyle --style=linux --indent=force-tab=8 --align-pointer=name $1 >>> $DIR/cleanfile $1 >>> $DIR/checkpatch.pl -f $1 >>> -- >>> 2.1.0 >>> >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >> >> > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs > > >
On 25 March 2015 at 07:43, Bill Fischofer <bill.fischofer@linaro.org> wrote: > If it's changing files, it shouldn't be (misleadingly) named "check", > which implies that it's just examining them. odp_fixwhitespace perhaps? > It is both but we can change the name, no problem but to me fixwihitespace is the minor feature. For me the checkptach element is the most valuable, then I dont need to make a patch before seeing that I have something to fix, I guess it could just warn you that checkpatch does not like the white space, but then you would have to go fix it by hand. odp_clean_and_check ? Mike > > On Wed, Mar 25, 2015 at 6:29 AM, Mike Holmes <mike.holmes@linaro.org> > wrote: > >> >> >> On 24 March 2015 at 18:20, Bill Fischofer <bill.fischofer@linaro.org> >> wrote: >> >>> General comment: Not clear whether this is checking (sort of implied by >>> the name odp_check) or changing files to match a predefined style. If the >>> latter then some other name should be used as I wouldn't expect a "check" >>> function to write anything except diagnostics. Some clarity on intended >>> use would help here. >>> >> >> It used to reformat fully - be we have not adopted that, so after this >> patch it just fixes whitespace and does checkpatch equivalent but on a >> given file rather than a patch. >> >> >>> >>> On Wed, Mar 18, 2015 at 7:17 PM, Mike Holmes <mike.holmes@linaro.org> >>> wrote: >>> >>>> ODP has not adopted a style that can be universally applied with a tool >>>> such as astyle. >>>> Remove astyle leaving only the cleanup for whitespace and checkpatch >>>> elements for checking src files before a patch is created. >>>> >>>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >>>> --- >>>> scripts/odp_check | 10 +--------- >>>> 1 file changed, 1 insertion(+), 9 deletions(-) >>>> >>>> diff --git a/scripts/odp_check b/scripts/odp_check >>>> index 09c859b..33809dc 100755 >>>> --- a/scripts/odp_check >>>> +++ b/scripts/odp_check >>>> @@ -1,8 +1,6 @@ >>>> #!/bin/bash >>>> # >>>> -# This script is an indenter, white space remover, >>>> -# formatter, and beautifier and general source file >>>> -# clean up for the ODP project. >>>> +# This script is a clean up for the ODP project src files. >>>> # >>>> # Usage >>>> # ./scripts/opd_check <path/filename> >>>> >>> >>> Typo: opd_check instead of odp_check >>> >>> >>>> @@ -10,11 +8,5 @@ set -e >>>> >>>> DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" >>>> >>>> -if ! type "astyle" >/dev/null >/dev/null; then >>>> - echo "Please install astyle from http://astyle.sourceforge.net/" >>>> - exit -1 >>>> -fi >>>> - >>>> -astyle --style=linux --indent=force-tab=8 --align-pointer=name $1 >>>> $DIR/cleanfile $1 >>>> $DIR/checkpatch.pl -f $1 >>>> -- >>>> 2.1.0 >>>> >>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>> >>> >>> >> >> >> -- >> Mike Holmes >> Technical Manager - Linaro Networking Group >> Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs >> >> >> >
odp_clean_and_check sounds good. As long as nobody is surprised to discover that it's (re)writing their files. On Wed, Mar 25, 2015 at 6:49 AM, Mike Holmes <mike.holmes@linaro.org> wrote: > > > On 25 March 2015 at 07:43, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > >> If it's changing files, it shouldn't be (misleadingly) named "check", >> which implies that it's just examining them. odp_fixwhitespace perhaps? >> > > It is both but we can change the name, no problem but to me fixwihitespace > is the minor feature. > > For me the checkptach element is the most valuable, then I dont need to > make a patch before seeing that I have something to fix, I guess it could > just warn you that checkpatch does not like the white space, but then you > would have to go fix it by hand. > > odp_clean_and_check ? > > Mike > > > > >> >> On Wed, Mar 25, 2015 at 6:29 AM, Mike Holmes <mike.holmes@linaro.org> >> wrote: >> >>> >>> >>> On 24 March 2015 at 18:20, Bill Fischofer <bill.fischofer@linaro.org> >>> wrote: >>> >>>> General comment: Not clear whether this is checking (sort of implied by >>>> the name odp_check) or changing files to match a predefined style. If the >>>> latter then some other name should be used as I wouldn't expect a "check" >>>> function to write anything except diagnostics. Some clarity on intended >>>> use would help here. >>>> >>> >>> It used to reformat fully - be we have not adopted that, so after this >>> patch it just fixes whitespace and does checkpatch equivalent but on a >>> given file rather than a patch. >>> >>> >>>> >>>> On Wed, Mar 18, 2015 at 7:17 PM, Mike Holmes <mike.holmes@linaro.org> >>>> wrote: >>>> >>>>> ODP has not adopted a style that can be universally applied with a tool >>>>> such as astyle. >>>>> Remove astyle leaving only the cleanup for whitespace and checkpatch >>>>> elements for checking src files before a patch is created. >>>>> >>>>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >>>>> --- >>>>> scripts/odp_check | 10 +--------- >>>>> 1 file changed, 1 insertion(+), 9 deletions(-) >>>>> >>>>> diff --git a/scripts/odp_check b/scripts/odp_check >>>>> index 09c859b..33809dc 100755 >>>>> --- a/scripts/odp_check >>>>> +++ b/scripts/odp_check >>>>> @@ -1,8 +1,6 @@ >>>>> #!/bin/bash >>>>> # >>>>> -# This script is an indenter, white space remover, >>>>> -# formatter, and beautifier and general source file >>>>> -# clean up for the ODP project. >>>>> +# This script is a clean up for the ODP project src files. >>>>> # >>>>> # Usage >>>>> # ./scripts/opd_check <path/filename> >>>>> >>>> >>>> Typo: opd_check instead of odp_check >>>> >>>> >>>>> @@ -10,11 +8,5 @@ set -e >>>>> >>>>> DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" >>>>> >>>>> -if ! type "astyle" >/dev/null >/dev/null; then >>>>> - echo "Please install astyle from http://astyle.sourceforge.net/" >>>>> - exit -1 >>>>> -fi >>>>> - >>>>> -astyle --style=linux --indent=force-tab=8 --align-pointer=name $1 >>>>> $DIR/cleanfile $1 >>>>> $DIR/checkpatch.pl -f $1 >>>>> -- >>>>> 2.1.0 >>>>> >>>>> >>>>> _______________________________________________ >>>>> lng-odp mailing list >>>>> lng-odp@lists.linaro.org >>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>> >>>> >>>> >>> >>> >>> -- >>> Mike Holmes >>> Technical Manager - Linaro Networking Group >>> Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM >>> SoCs >>> >>> >>> >> > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs > > >
diff --git a/scripts/odp_check b/scripts/odp_check index 09c859b..33809dc 100755 --- a/scripts/odp_check +++ b/scripts/odp_check @@ -1,8 +1,6 @@ #!/bin/bash # -# This script is an indenter, white space remover, -# formatter, and beautifier and general source file -# clean up for the ODP project. +# This script is a clean up for the ODP project src files. # # Usage # ./scripts/opd_check <path/filename> @@ -10,11 +8,5 @@ set -e DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" -if ! type "astyle" >/dev/null >/dev/null; then - echo "Please install astyle from http://astyle.sourceforge.net/" - exit -1 -fi - -astyle --style=linux --indent=force-tab=8 --align-pointer=name $1 $DIR/cleanfile $1 $DIR/checkpatch.pl -f $1
ODP has not adopted a style that can be universally applied with a tool such as astyle. Remove astyle leaving only the cleanup for whitespace and checkpatch elements for checking src files before a patch is created. Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- scripts/odp_check | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)