mbox series

[v4,0/6] refine argparse library

Message ID 20240318111838.16991-1-fengchengwen@huawei.com (mailing list archive)
Headers
Series refine argparse library |

Message

fengchengwen March 18, 2024, 11:18 a.m. UTC 
I found a couple of issues when I revisited the argparse_autotest
output, so got this patchset.

Chengwen Feng (6):
  argparse: refine error message
  argparse: remove dead code
  argparse: replace flag enum with marco
  argparse: fix argument flags operate as uint32 type
  test/argparse: refine testcases
  argparse: fix doc don't display two hyphens

---
v4: address Thomas's comment on 4/6 commit:
- remove unused macros TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK and
  TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK.
v3:
- address Thomas's comment on 4/6 comit.
- add commit: fix doc don't display two hyphens.
v2: address David Marchand's comment:
- replace flag enum with marco.
- replace flag's hardcode with macro in test_argparse.c.

 app/test/test_argparse.c               | 65 +++++++++++---------
 doc/guides/prog_guide/argparse_lib.rst | 47 +++++++-------
 lib/argparse/rte_argparse.c            | 61 +++++++++---------
 lib/argparse/rte_argparse.h            | 85 ++++++++++++--------------
 4 files changed, 127 insertions(+), 131 deletions(-)

Comments

Thomas Monjalon March 21, 2024, 5:27 p.m. UTC | #1 
18/03/2024 12:18, Chengwen Feng:
> I found a couple of issues when I revisited the argparse_autotest
> output, so got this patchset.
> 
> Chengwen Feng (6):
>   argparse: refine error message
>   argparse: remove dead code
>   argparse: replace flag enum with marco
>   argparse: fix argument flags operate as uint32 type
>   test/argparse: refine testcases
>   argparse: fix doc don't display two hyphens

I'd like to get an approval from someone else for the API changes here.
I feel it is better to wait post 24.03.
Anyway it is a fresh experimental API.
fengchengwen March 22, 2024, 1:24 a.m. UTC | #2 
Hi Thomas,

On 2024/3/22 1:27, Thomas Monjalon wrote:
> 18/03/2024 12:18, Chengwen Feng:
>> I found a couple of issues when I revisited the argparse_autotest
>> output, so got this patchset.
>>
>> Chengwen Feng (6):
>>   argparse: refine error message
>>   argparse: remove dead code
>>   argparse: replace flag enum with marco
>>   argparse: fix argument flags operate as uint32 type
>>   test/argparse: refine testcases
>>   argparse: fix doc don't display two hyphens
> 
> I'd like to get an approval from someone else for the API changes here.
> I feel it is better to wait post 24.03.
> Anyway it is a fresh experimental API.

That's OK

Thanks

> 
> 
> 
> .
>
fengchengwen May 8, 2024, 9:10 a.m. UTC | #3 
Hi Thomas,

On 2024/3/22 1:27, Thomas Monjalon wrote:
> 18/03/2024 12:18, Chengwen Feng:
>> I found a couple of issues when I revisited the argparse_autotest
>> output, so got this patchset.
>>
>> Chengwen Feng (6):
>>   argparse: refine error message
>>   argparse: remove dead code
>>   argparse: replace flag enum with marco
>>   argparse: fix argument flags operate as uint32 type
>>   test/argparse: refine testcases
>>   argparse: fix doc don't display two hyphens
> 
> I'd like to get an approval from someone else for the API changes here.
> I feel it is better to wait post 24.03.
> Anyway it is a fresh experimental API.

Could you review this patchset and merge?
BTW: this patchset also fixes bug 1409

Thanks

> 
> 
> 
> .
>
fengchengwen July 4, 2024, 3:07 a.m. UTC | #4 
Kindly ping.

Best regards,
Chengwen Feng

On 2024/3/18 19:18, Chengwen Feng wrote:
> I found a couple of issues when I revisited the argparse_autotest
> output, so got this patchset.
> 
> Chengwen Feng (6):
>   argparse: refine error message
>   argparse: remove dead code
>   argparse: replace flag enum with marco
>   argparse: fix argument flags operate as uint32 type
>   test/argparse: refine testcases
>   argparse: fix doc don't display two hyphens
> 
> ---
> v4: address Thomas's comment on 4/6 commit:
> - remove unused macros TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK and
>   TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK.
> v3:
> - address Thomas's comment on 4/6 comit.
> - add commit: fix doc don't display two hyphens.
> v2: address David Marchand's comment:
> - replace flag enum with marco.
> - replace flag's hardcode with macro in test_argparse.c.
> 
>  app/test/test_argparse.c               | 65 +++++++++++---------
>  doc/guides/prog_guide/argparse_lib.rst | 47 +++++++-------
>  lib/argparse/rte_argparse.c            | 61 +++++++++---------
>  lib/argparse/rte_argparse.h            | 85 ++++++++++++--------------
>  4 files changed, 127 insertions(+), 131 deletions(-)
>
Jie Hai July 5, 2024, 6:06 a.m. UTC | #5 
For the patchset,
Acked-by: Jie Hai <haijie1@huawei.com>

On 2024/3/18 19:18, Chengwen Feng wrote:
> I found a couple of issues when I revisited the argparse_autotest
> output, so got this patchset.
> 
> Chengwen Feng (6):
>    argparse: refine error message
>    argparse: remove dead code
>    argparse: replace flag enum with marco
>    argparse: fix argument flags operate as uint32 type
>    test/argparse: refine testcases
>    argparse: fix doc don't display two hyphens
> ---
> v4: address Thomas's comment on 4/6 commit:
> - remove unused macros TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK and
>    TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK.
> v3:
> - address Thomas's comment on 4/6 comit.
> - add commit: fix doc don't display two hyphens.
> v2: address David Marchand's comment:
> - replace flag enum with marco.
> - replace flag's hardcode with macro in test_argparse.c.
> 
>   app/test/test_argparse.c               | 65 +++++++++++---------
>   doc/guides/prog_guide/argparse_lib.rst | 47 +++++++-------
>   lib/argparse/rte_argparse.c            | 61 +++++++++---------
>   lib/argparse/rte_argparse.h            | 85 ++++++++++++--------------
>   4 files changed, 127 insertions(+), 131 deletions(-)
>
Dengdui Huang July 9, 2024, 10:53 a.m. UTC | #6 
For the patchset,
Reviewed-by: Dengdui Huang <huangdengdui@huawei.com>

On 2024/3/18 19:18, Chengwen Feng wrote:
> I found a couple of issues when I revisited the argparse_autotest
> output, so got this patchset.
> 
> Chengwen Feng (6):
>   argparse: refine error message
>   argparse: remove dead code
>   argparse: replace flag enum with marco
>   argparse: fix argument flags operate as uint32 type
>   test/argparse: refine testcases
>   argparse: fix doc don't display two hyphens
> 
> ---
> v4: address Thomas's comment on 4/6 commit:
> - remove unused macros TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK and
>   TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK.
> v3:
> - address Thomas's comment on 4/6 comit.
> - add commit: fix doc don't display two hyphens.
> v2: address David Marchand's comment:
> - replace flag enum with marco.
> - replace flag's hardcode with macro in test_argparse.c.
> 
>  app/test/test_argparse.c               | 65 +++++++++++---------
>  doc/guides/prog_guide/argparse_lib.rst | 47 +++++++-------
>  lib/argparse/rte_argparse.c            | 61 +++++++++---------
>  lib/argparse/rte_argparse.h            | 85 ++++++++++++--------------
>  4 files changed, 127 insertions(+), 131 deletions(-)
>
fengchengwen Sept. 6, 2024, 12:59 a.m. UTC | #7 
Hi Thomas and David,

This patchset is good enhance for argparse library, we hope it could upstream to DPDK 24.11.
Please look at it.

Thanks

On 2024/7/4 11:07, fengchengwen wrote:
> Kindly ping.
> 
> Best regards,
> Chengwen Feng
> 
> On 2024/3/18 19:18, Chengwen Feng wrote:
>> I found a couple of issues when I revisited the argparse_autotest
>> output, so got this patchset.
>>
>> Chengwen Feng (6):
>>   argparse: refine error message
>>   argparse: remove dead code
>>   argparse: replace flag enum with marco
>>   argparse: fix argument flags operate as uint32 type
>>   test/argparse: refine testcases
>>   argparse: fix doc don't display two hyphens
>>
>> ---
>> v4: address Thomas's comment on 4/6 commit:
>> - remove unused macros TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK and
>>   TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK.
>> v3:
>> - address Thomas's comment on 4/6 comit.
>> - add commit: fix doc don't display two hyphens.
>> v2: address David Marchand's comment:
>> - replace flag enum with marco.
>> - replace flag's hardcode with macro in test_argparse.c.
>>
>>  app/test/test_argparse.c               | 65 +++++++++++---------
>>  doc/guides/prog_guide/argparse_lib.rst | 47 +++++++-------
>>  lib/argparse/rte_argparse.c            | 61 +++++++++---------
>>  lib/argparse/rte_argparse.h            | 85 ++++++++++++--------------
>>  4 files changed, 127 insertions(+), 131 deletions(-)
>>
> .
>
Stephen Hemminger Oct. 7, 2024, 8:32 p.m. UTC | #8 
On Mon, 18 Mar 2024 11:18:32 +0000
Chengwen Feng <fengchengwen@huawei.com> wrote:

> I found a couple of issues when I revisited the argparse_autotest
> output, so got this patchset.
> 
> Chengwen Feng (6):
>   argparse: refine error message
>   argparse: remove dead code
>   argparse: replace flag enum with marco
>   argparse: fix argument flags operate as uint32 type
>   test/argparse: refine testcases
>   argparse: fix doc don't display two hyphens
> 
> ---
> v4: address Thomas's comment on 4/6 commit:
> - remove unused macros TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK and
>   TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK.
> v3:
> - address Thomas's comment on 4/6 comit.
> - add commit: fix doc don't display two hyphens.
> v2: address David Marchand's comment:
> - replace flag enum with marco.
> - replace flag's hardcode with macro in test_argparse.c.
> 
>  app/test/test_argparse.c               | 65 +++++++++++---------
>  doc/guides/prog_guide/argparse_lib.rst | 47 +++++++-------
>  lib/argparse/rte_argparse.c            | 61 +++++++++---------
>  lib/argparse/rte_argparse.h            | 85 ++++++++++++--------------
>  4 files changed, 127 insertions(+), 131 deletions(-)
> 


The patch set looks good.
My only comment is the term 'refine' was confusing to me.
I guess you meant "improve", "clarify" or "reword" since most
of the changes are about changing the wording.

Series-Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Stephen Hemminger Oct. 9, 2024, 7:33 p.m. UTC | #9 
On Mon, 18 Mar 2024 11:18:32 +0000
Chengwen Feng <fengchengwen@huawei.com> wrote:

> I found a couple of issues when I revisited the argparse_autotest
> output, so got this patchset.
> 
> Chengwen Feng (6):
>   argparse: refine error message
>   argparse: remove dead code
>   argparse: replace flag enum with marco
>   argparse: fix argument flags operate as uint32 type
>   test/argparse: refine testcases
>   argparse: fix doc don't display two hyphens
> 
> ---
> v4: address Thomas's comment on 4/6 commit:
> - remove unused macros TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK and
>   TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK.
> v3:
> - address Thomas's comment on 4/6 comit.
> - add commit: fix doc don't display two hyphens.
> v2: address David Marchand's comment:
> - replace flag enum with marco.
> - replace flag's hardcode with macro in test_argparse.c.
> 
>  app/test/test_argparse.c               | 65 +++++++++++---------
>  doc/guides/prog_guide/argparse_lib.rst | 47 +++++++-------
>  lib/argparse/rte_argparse.c            | 61 +++++++++---------
>  lib/argparse/rte_argparse.h            | 85 ++++++++++++--------------
>  4 files changed, 127 insertions(+), 131 deletions(-)

The verb refine is not usually used for edits to documents or software
that probably is a translation issue.

Series-acked-by: Stephen Hemminger <stephen@networkplumber.org>
fengchengwen Oct. 10, 2024, 1:35 a.m. UTC | #10 
Hi Thomas and David,

Kindly ping for merge.

On 2024/10/10 3:33, Stephen Hemminger wrote:
> On Mon, 18 Mar 2024 11:18:32 +0000
> Chengwen Feng <fengchengwen@huawei.com> wrote:
> 
>> I found a couple of issues when I revisited the argparse_autotest
>> output, so got this patchset.
>>
>> Chengwen Feng (6):
>>   argparse: refine error message
>>   argparse: remove dead code
>>   argparse: replace flag enum with marco
>>   argparse: fix argument flags operate as uint32 type
>>   test/argparse: refine testcases
>>   argparse: fix doc don't display two hyphens
>>
>> ---
>> v4: address Thomas's comment on 4/6 commit:
>> - remove unused macros TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK and
>>   TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK.
>> v3:
>> - address Thomas's comment on 4/6 comit.
>> - add commit: fix doc don't display two hyphens.
>> v2: address David Marchand's comment:
>> - replace flag enum with marco.
>> - replace flag's hardcode with macro in test_argparse.c.
>>
>>  app/test/test_argparse.c               | 65 +++++++++++---------
>>  doc/guides/prog_guide/argparse_lib.rst | 47 +++++++-------
>>  lib/argparse/rte_argparse.c            | 61 +++++++++---------
>>  lib/argparse/rte_argparse.h            | 85 ++++++++++++--------------
>>  4 files changed, 127 insertions(+), 131 deletions(-)
> 
> The verb refine is not usually used for edits to documents or software
> that probably is a translation issue.
> 
> Series-acked-by: Stephen Hemminger <stephen@networkplumber.org>

Thank Stephen
fengchengwen Oct. 18, 2024, 1:09 a.m. UTC | #11 
Hi Thomas and David,

This patchset was already acked by Stephen.
It missed in DPDK 24.07, hope merge in 24.11.

Thanks

On 2024/10/10 9:35, fengchengwen wrote:
> Hi Thomas and David,
> 
> Kindly ping for merge.
> 
> On 2024/10/10 3:33, Stephen Hemminger wrote:
>> On Mon, 18 Mar 2024 11:18:32 +0000
>> Chengwen Feng <fengchengwen@huawei.com> wrote:
>>
>>> I found a couple of issues when I revisited the argparse_autotest
>>> output, so got this patchset.
>>>
>>> Chengwen Feng (6):
>>>   argparse: refine error message
>>>   argparse: remove dead code
>>>   argparse: replace flag enum with marco
>>>   argparse: fix argument flags operate as uint32 type
>>>   test/argparse: refine testcases
>>>   argparse: fix doc don't display two hyphens
>>>
>>> ---
>>> v4: address Thomas's comment on 4/6 commit:
>>> - remove unused macros TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK and
>>>   TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK.
>>> v3:
>>> - address Thomas's comment on 4/6 comit.
>>> - add commit: fix doc don't display two hyphens.
>>> v2: address David Marchand's comment:
>>> - replace flag enum with marco.
>>> - replace flag's hardcode with macro in test_argparse.c.
>>>
>>>  app/test/test_argparse.c               | 65 +++++++++++---------
>>>  doc/guides/prog_guide/argparse_lib.rst | 47 +++++++-------
>>>  lib/argparse/rte_argparse.c            | 61 +++++++++---------
>>>  lib/argparse/rte_argparse.h            | 85 ++++++++++++--------------
>>>  4 files changed, 127 insertions(+), 131 deletions(-)
>>
>> The verb refine is not usually used for edits to documents or software
>> that probably is a translation issue.
>>
>> Series-acked-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> Thank Stephen
>
Thomas Monjalon Oct. 18, 2024, 2:46 p.m. UTC | #12 
18/10/2024 03:09, fengchengwen:
> Hi Thomas and David,
> 
> This patchset was already acked by Stephen.
> It missed in DPDK 24.07, hope merge in 24.11.
> 
> Thanks
> 
> On 2024/10/10 9:35, fengchengwen wrote:
> > Hi Thomas and David,
> > 
> > Kindly ping for merge.
> > 
> > On 2024/10/10 3:33, Stephen Hemminger wrote:
> >> On Mon, 18 Mar 2024 11:18:32 +0000
> >> Chengwen Feng <fengchengwen@huawei.com> wrote:
> >>
> >>> I found a couple of issues when I revisited the argparse_autotest
> >>> output, so got this patchset.
> >>>
> >>> Chengwen Feng (6):
> >>>   argparse: refine error message
> >>>   argparse: remove dead code
> >>>   argparse: replace flag enum with marco
> >>>   argparse: fix argument flags operate as uint32 type
> >>>   test/argparse: refine testcases
> >>>   argparse: fix doc don't display two hyphens
> >>>
> >>> ---
> >>> v4: address Thomas's comment on 4/6 commit:
> >>> - remove unused macros TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK and
> >>>   TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK.
> >>> v3:
> >>> - address Thomas's comment on 4/6 comit.
> >>> - add commit: fix doc don't display two hyphens.
> >>> v2: address David Marchand's comment:
> >>> - replace flag enum with marco.
> >>> - replace flag's hardcode with macro in test_argparse.c.
> >>>
> >>>  app/test/test_argparse.c               | 65 +++++++++++---------
> >>>  doc/guides/prog_guide/argparse_lib.rst | 47 +++++++-------
> >>>  lib/argparse/rte_argparse.c            | 61 +++++++++---------
> >>>  lib/argparse/rte_argparse.h            | 85 ++++++++++++--------------
> >>>  4 files changed, 127 insertions(+), 131 deletions(-)
> >>
> >> The verb refine is not usually used for edits to documents or software
> >> that probably is a translation issue.
> >>
> >> Series-acked-by: Stephen Hemminger <stephen@networkplumber.org>
> > 
> > Thank Stephen

Added Bugzilla ID, CC: stable, reworded as Stephen suggests,
and applied.

Note that the lists in doxygen comments does not look nice.
You may look at this: https://www.doxygen.nl/manual/lists.html
fengchengwen Oct. 21, 2024, 7 a.m. UTC | #13 
On 2024/10/18 22:46, Thomas Monjalon wrote:
> 18/10/2024 03:09, fengchengwen:
>> Hi Thomas and David,
>>
>> This patchset was already acked by Stephen.
>> It missed in DPDK 24.07, hope merge in 24.11.
>>
>> Thanks
>>
>> On 2024/10/10 9:35, fengchengwen wrote:
>>> Hi Thomas and David,
>>>
>>> Kindly ping for merge.
>>>
>>> On 2024/10/10 3:33, Stephen Hemminger wrote:
>>>> On Mon, 18 Mar 2024 11:18:32 +0000
>>>> Chengwen Feng <fengchengwen@huawei.com> wrote:
>>>>
>>>>> I found a couple of issues when I revisited the argparse_autotest
>>>>> output, so got this patchset.
>>>>>
>>>>> Chengwen Feng (6):
>>>>>   argparse: refine error message
>>>>>   argparse: remove dead code
>>>>>   argparse: replace flag enum with marco
>>>>>   argparse: fix argument flags operate as uint32 type
>>>>>   test/argparse: refine testcases
>>>>>   argparse: fix doc don't display two hyphens
>>>>>
>>>>> ---
>>>>> v4: address Thomas's comment on 4/6 commit:
>>>>> - remove unused macros TEST_ARGPARSE_FLAG_HAS_ARG_BITMASK and
>>>>>   TEST_ARGPARSE_FLAG_VAL_TYPE_BITMASK.
>>>>> v3:
>>>>> - address Thomas's comment on 4/6 comit.
>>>>> - add commit: fix doc don't display two hyphens.
>>>>> v2: address David Marchand's comment:
>>>>> - replace flag enum with marco.
>>>>> - replace flag's hardcode with macro in test_argparse.c.
>>>>>
>>>>>  app/test/test_argparse.c               | 65 +++++++++++---------
>>>>>  doc/guides/prog_guide/argparse_lib.rst | 47 +++++++-------
>>>>>  lib/argparse/rte_argparse.c            | 61 +++++++++---------
>>>>>  lib/argparse/rte_argparse.h            | 85 ++++++++++++--------------
>>>>>  4 files changed, 127 insertions(+), 131 deletions(-)
>>>>
>>>> The verb refine is not usually used for edits to documents or software
>>>> that probably is a translation issue.
>>>>
>>>> Series-acked-by: Stephen Hemminger <stephen@networkplumber.org>
>>>
>>> Thank Stephen
> 
> Added Bugzilla ID, CC: stable, reworded as Stephen suggests,
> and applied.

Thanks

> 
> Note that the lists in doxygen comments does not look nice.
> You may look at this: https://www.doxygen.nl/manual/lists.html

I'll look at it and push another commit if needed.

> 
>