diff mbox series

[1/2] devtools: report the incorrect section when complaining

Message ID 1543400933-1723-1-git-send-email-david.marchand@redhat.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers
Series [1/2] devtools: report the incorrect section when complaining |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

David Marchand Nov. 28, 2018, 10:28 a.m. UTC 
  It does not hurt reporting the incriminated section.

Before:
ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in a
section other than the EXPERIMENTAL section of the version map

After:
ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in
+EXPERIMENTAL section other than the EXPERIMENTAL section of the
version map

Signed-off-by: David Marchand <david.marchand@redhat.com>
---

Used http://patchwork.dpdk.org/patch/48354/ to test.

---
 devtools/check-symbol-change.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Neil Horman Nov. 28, 2018, 12:34 p.m. UTC | #1 
On Wed, Nov 28, 2018 at 11:28:52AM +0100, David Marchand wrote:
> It does not hurt reporting the incriminated section.
> 
> Before:
> ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in a
> section other than the EXPERIMENTAL section of the version map
> 
> After:
> ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in
> +EXPERIMENTAL section other than the EXPERIMENTAL section of the
> version map
> 
nit: Its a bit odd in the changelog to have an example in which the incorect
section being reported matches the expected section.  I.e. its confusing to read
"... is added in +EXPERIMENTAL section other than the EXPERIMENTAL section".
Might be better to change the language of the report below and the example to be
something like:

ERROR: symbol <SYMBOL> is added in the <VERSION> section, but is expected to be
added in the EXPERIMENTAL section

ACK to the notion of reporting the offending section though.  Thats a good idea.

Neil

> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> 
> Used http://patchwork.dpdk.org/patch/48354/ to test.
> 
> ---
>  devtools/check-symbol-change.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
> index 1d21e91..66741be 100755
> --- a/devtools/check-symbol-change.sh
> +++ b/devtools/check-symbol-change.sh
> @@ -115,7 +115,7 @@ check_for_rule_violations()
>  				if [ $? -ne 0 ]
>  				then
>  					echo -n "ERROR: symbol $symname "
> -					echo -n "is added in a section "
> +					echo -n "is added in $secname section "
>  					echo -n "other than the EXPERIMENTAL "
>  					echo "section of the version map"
>  					ret=1
> -- 
> 1.8.3.1
> 
>
David Marchand Nov. 28, 2018, 1:07 p.m. UTC | #2 
On Wed, Nov 28, 2018 at 1:35 PM Neil Horman <nhorman@tuxdriver.com> wrote:

> On Wed, Nov 28, 2018 at 11:28:52AM +0100, David Marchand wrote:
> > It does not hurt reporting the incriminated section.
> >
> > Before:
> > ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in a
> > section other than the EXPERIMENTAL section of the version map
> >
> > After:
> > ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in
> > +EXPERIMENTAL section other than the EXPERIMENTAL section of the
> > version map
> >
> nit: Its a bit odd in the changelog to have an example in which the
> incorect
> section being reported matches the expected section.  I.e. its confusing
> to read
> "... is added in +EXPERIMENTAL section other than the EXPERIMENTAL
> section".
> Might be better to change the language of the report below and the example
> to be
> something like:
>
> ERROR: symbol <SYMBOL> is added in the <VERSION> section, but is expected
> to be
> added in the EXPERIMENTAL section
>
> ACK to the notion of reporting the offending section though.  Thats a good
> idea.
>

Ok, updated for v2.
Neil Horman Nov. 28, 2018, 9:23 p.m. UTC | #3 
On Wed, Nov 28, 2018 at 02:07:25PM +0100, David Marchand wrote:
> On Wed, Nov 28, 2018 at 1:35 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > On Wed, Nov 28, 2018 at 11:28:52AM +0100, David Marchand wrote:
> > > It does not hurt reporting the incriminated section.
> > >
> > > Before:
> > > ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in a
> > > section other than the EXPERIMENTAL section of the version map
> > >
> > > After:
> > > ERROR: symbol rte_meter_trtcm_rfc4115_color_aware_check is added in
> > > +EXPERIMENTAL section other than the EXPERIMENTAL section of the
> > > version map
> > >
> > nit: Its a bit odd in the changelog to have an example in which the
> > incorect
> > section being reported matches the expected section.  I.e. its confusing
> > to read
> > "... is added in +EXPERIMENTAL section other than the EXPERIMENTAL
> > section".
> > Might be better to change the language of the report below and the example
> > to be
> > something like:
> >
> > ERROR: symbol <SYMBOL> is added in the <VERSION> section, but is expected
> > to be
> > added in the EXPERIMENTAL section
> >
> > ACK to the notion of reporting the offending section though.  Thats a good
> > idea.
> >
> 
> Ok, updated for v2.
> 
Thanks!
Neil

> -- 
> David Marchand
diff mbox series

Patch

diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh
index 1d21e91..66741be 100755
--- a/devtools/check-symbol-change.sh
+++ b/devtools/check-symbol-change.sh
@@ -115,7 +115,7 @@  check_for_rule_violations()
 				if [ $? -ne 0 ]
 				then
 					echo -n "ERROR: symbol $symname "
-					echo -n "is added in a section "
+					echo -n "is added in $secname section "
 					echo -n "other than the EXPERIMENTAL "
 					echo "section of the version map"
 					ret=1