[
https://issues.apache.org/jira/browse/MINIFICPP-1203?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Adam Hunyadi updated MINIFICPP-1203:
------------------------------------
Description:
*Background:*
Currently cpplink checks are not checking all the files correctly and are
hiding some errors that are meant to be displayed. Manually running the
following cpplint check shows overreports the number of errors but it is a
decent estimate on the number of errors ignored:
{code:bash}
# This command shows some errors that we otherwise suppress in the project
cpplint --linelength=200
filter=-runtime/reference,-runtime/string,-build/c++11,-build/include_order,-build/include_alpha
`find libminifi/ -name \.cpp -o -name \*.h`
(...)
Total errors found: 1730
{code}
When running {{{color:#403294}{{make linter}}{color}}} these errors are
supressed. It runs the following command in
{{{color:#403294}run_linter.sh{color}}}:
{code:bash}
python ${SCRIPT_DIR}/cpplint.py --linelength=200 --headers=${HEADERS} ${SOURCES}
{code}
For some reason, it seems like the files specified in the
{{{color:#403294}{{--headers}}{color}}} flag are ignored altogether. For example
{code:bash}
# Running w/ headers option set
cpplint --filter="-runtime/reference,-runtime/string,-build/c++11"
--linelength=200 --headers=`find . -name "*.h" | tr '\n' ','`
libminifi/include/processors/ProcessorUtils.h 2>/dev/null
Done processing libminifi/include/processors/ProcessorUtils.h
# Running w/ unspecified headers
cpplint --filter="-runtime/reference,-runtime/string,-build/c++11"
--linelength=200 libminifi/include/processors/ProcessorUtils.h 2>/dev/null
Done processing libminifi/include/processors/ProcessorUtils.h
Total errors found: 6
{code}
*Proposal:*
We should remove the header specification from {{{color:#403294}{{make
linter}}{color}}} and set up linter configuration files in the project
directories that set all the rules to be applied on the specific directory
contents recursively.
There is to approach for doing this: we can either specify files or rules to be
ignored when doing the linter check. The latter is preferable, so that when we
want to clear them up later, we can have separate commits/pull request for each
of the warning fixed (and potentially automatize fixes (eg. writing clang-tidy
rules or applying linter fixes).
(!) The commits on this Jira are not expected to fix any warnings reported by
the linter, but to have all the checks disabled.
*Update:*
We decided to replace header guards with {{{color:#403294}{{#pragma
once}}{color}}}. It is not standardized, but all the compilers we support have
it, and we already have it scattered in our headers, so we can consider this
update safe. This is now extracted into its own Jira (see related).
was:
*Background:*
Currently cpplink checks are not checking all the files correctly and are
hiding some errors that are meant to be displayed. Manually running the
following cpplint check shows overreports the number of errors but it is a
decent estimate on the number of errors ignored:
{code:bash}
# This command shows some errors that we otherwise suppress in the project
cpplint --linelength=200
filter=-runtime/reference,-runtime/string,-build/c++11,-build/include_order,-build/include_alpha
`find libminifi/ -name \.cpp -o -name \*.h`
(...)
Total errors found: 1730
{code}
When running {{{color:#403294}{{make linter}}{color}}} these errors are
supressed. It runs the following command in
{{{color:#403294}run_linter.sh{color}}}:
{code:bash}
python ${SCRIPT_DIR}/cpplint.py --linelength=200 --headers=${HEADERS} ${SOURCES}
{code}
For some reason, it seems like the files specified in the
{{{color:#403294}{{--headers}}{color}}} flag are ignored altogether. For example
{code:bash}
# Running w/ headers option set
cpplint --filter="-runtime/reference,-runtime/string,-build/c++11"
--linelength=200 --headers=`find . -name "*.h" | tr '\n' ','`
libminifi/include/processors/ProcessorUtils.h 2>/dev/null
Done processing libminifi/include/processors/ProcessorUtils.h
# Running w/ unspecified headers
cpplint --filter="-runtime/reference,-runtime/string,-build/c++11"
--linelength=200 libminifi/include/processors/ProcessorUtils.h 2>/dev/null
Done processing libminifi/include/processors/ProcessorUtils.h
Total errors found: 6
{code}
*Proposal:*
We should remove the header specification from {{{color:#403294}{{make
linter}}{color}}} and set up linter configuration files in the project
directories that set all the rules to be applied on the specific directory
contents recursively.
There is to approach for doing this: we can either specify files or rules to be
ignored when doing the linter check. The latter is preferable, so that when we
want to clear them up later, we can have separate commits/pull request for each
of the warning fixed (and potentially automatize fixes (eg. writing clang-tidy
rules or applying linter fixes).
(!) The commits on this Jira are not expected to fix any warnings reported by
the linter, but to have all the checks disabled.
*Update:*
We decided to replace header guards with {{{color:#403294}{{#pragma
once}}{color}}}. It is not standardized, but all the compilers we support have
it, and we already have it scattered in our headers, so we can consider this
update safe.
> Set up cpplint so that it can be configured per-directory
> ---------------------------------------------------------
>
> Key: MINIFICPP-1203
> URL: https://issues.apache.org/jira/browse/MINIFICPP-1203
> Project: Apache NiFi MiNiFi C++
> Issue Type: Bug
> Affects Versions: 0.7.0
> Reporter: Adam Hunyadi
> Assignee: Adam Hunyadi
> Priority: Minor
> Labels: MiNiFi-CPP-Hygiene
> Fix For: 0.8.0
>
> Time Spent: 18h
> Remaining Estimate: 0h
>
> *Background:*
> Currently cpplink checks are not checking all the files correctly and are
> hiding some errors that are meant to be displayed. Manually running the
> following cpplint check shows overreports the number of errors but it is a
> decent estimate on the number of errors ignored:
> {code:bash}
> # This command shows some errors that we otherwise suppress in the project
> cpplint --linelength=200
> filter=-runtime/reference,-runtime/string,-build/c++11,-build/include_order,-build/include_alpha
> `find libminifi/ -name \.cpp -o -name \*.h`
> (...)
> Total errors found: 1730
> {code}
> When running {{{color:#403294}{{make linter}}{color}}} these errors are
> supressed. It runs the following command in
> {{{color:#403294}run_linter.sh{color}}}:
> {code:bash}
> python ${SCRIPT_DIR}/cpplint.py --linelength=200 --headers=${HEADERS}
> ${SOURCES}
> {code}
> For some reason, it seems like the files specified in the
> {{{color:#403294}{{--headers}}{color}}} flag are ignored altogether. For
> example
> {code:bash}
> # Running w/ headers option set
> cpplint --filter="-runtime/reference,-runtime/string,-build/c++11"
> --linelength=200 --headers=`find . -name "*.h" | tr '\n' ','`
> libminifi/include/processors/ProcessorUtils.h 2>/dev/null
> Done processing libminifi/include/processors/ProcessorUtils.h
> # Running w/ unspecified headers
> cpplint --filter="-runtime/reference,-runtime/string,-build/c++11"
> --linelength=200 libminifi/include/processors/ProcessorUtils.h 2>/dev/null
> Done processing libminifi/include/processors/ProcessorUtils.h
> Total errors found: 6
> {code}
> *Proposal:*
> We should remove the header specification from {{{color:#403294}{{make
> linter}}{color}}} and set up linter configuration files in the project
> directories that set all the rules to be applied on the specific directory
> contents recursively.
> There is to approach for doing this: we can either specify files or rules to
> be ignored when doing the linter check. The latter is preferable, so that
> when we want to clear them up later, we can have separate commits/pull
> request for each of the warning fixed (and potentially automatize fixes (eg.
> writing clang-tidy rules or applying linter fixes).
> (!) The commits on this Jira are not expected to fix any warnings reported by
> the linter, but to have all the checks disabled.
> *Update:*
> We decided to replace header guards with {{{color:#403294}{{#pragma
> once}}{color}}}. It is not standardized, but all the compilers we support
> have it, and we already have it scattered in our headers, so we can consider
> this update safe. This is now extracted into its own Jira (see related).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
