Re: [PATCH v10 0/5] add unit tests for bitrate, latency and pdump libraries

[Thomas Monjalon <thomas@monjalon.net>]

03/08/2018 16:16, Parthasarathy, JananeeX M:
> Hi Thomas,
> 
> From: Pattan, Reshma
> >
> >Hi Thomas,
> >
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> >> 01/08/2018 00:18, Reshma Pattan:
> >> > v10: fixed clang compiler issues and freed latency stats memzone in
> >> > latency stats unit tests.
> >> > v9: rebased ontop of latest autotest changes and added new tests to
> >> > the autotest list
> >> > v8: renamed commit headline and freed the metrics memzone for
> >> > bitrate ut
> >> > v7: removed unused macros and corrected the comment
> >> > v6: updated ring variable appropriately
> >> > v5: rebased, freed pools and rings, created common patch set
> >> > ---
> >>
> >> Sorry, the integration of this patchset is very painful.
> >>
> >> After asking for rebase, for clang fix, there are still some basic
> >> errors with 32- bit compilation:
> >>
> >> 	test_latencystats.c:131:21: error:
> >> 	format ‘%ld’ expects argument of type ‘long int’,
> >> 	but argument 2 has type ‘uint64_t’ {aka ‘long long unsigned int’}
> >>
> >> linkage:
> >>
> >> 	test@test@@dpdk-test@exe/test.c.o:(.data+0x18): undefined
> >reference
> >> to `test_pdump'
> >>
> >> or even MAINTAINERS file:
> >>
> >> 	test/test/sample_packet_forward.c
> >> 	test/test/sample_packet_forward.h
> >> 	test/test/test_bitratestats.c
> >> 	test/test/test_latencystats.c
> >>
> >> I have already spent too much time on it, despite it is not fixing 18.08.
> >>
> >> Please do a complete detailed review of this series, so it can be
> >> considered for 18.11.
> >>
> >
> >We missed to do these basic checks, apologies for consuming your time.
> > Naga Suresh has now proactively worked on fixing these issues and running
> >pre checks on patches and addressed in v12.
> >The earlier versions were reviewed by me, Remy and Anatoly . So we request
> >you to consider latest patches for 18.08, until unless they don’t give any last
> >minute  surprises.
> >
> >Thanks,
> >Reshma
> >
> >
> >
> Apologies very much to miss the earlier patch pre-checks.
> We have gone through the cheatsheet and validated pre-checks in the patch v12.
> Compiled Successfully in Fedora 27, Fedora 26, CentOS 7.2, CentOS 7.4, Ubuntu for both 32bit/64bit and FreeBSD (64bit)
> Build using compilers gcc, icc, clang were successful in Fedora.
> Shared Library builds were successful in Fedora 27, CentOS 7.2 and Ubuntu
> 
> Executed checkpatch, check-git-log without any errors.
> Code changes were acknowledged by reviewers.
> 
> Request to please consider the patch set v12 to be included in RC3 18.08.
> 
> In case of any info/change please let us know.

Why are you insisting so much?
I have already replied to Reshma that it is too late and not urgent:
	http://mails.dpdk.org/archives/dev/2018-August/109352.html

Please do not make it even more difficult.
I just do not want to spend more time on this series now.

When I will take time, I will do a better review, and I can promise
that I will have some comments.
So please consider my earlier comment to avoid burning too much time:
	"Please do a complete detailed review of this series"

After a quick look, I already see some suspicious includes, linkage,
and last 2 patches should be integrated with others.

To make it clear, the quality was not good and I already burnt too much time.
I won't spend more time on it during August.