Thread (32 messages) 32 messages, 5 authors, 2018-08-24

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

From: Thomas Monjalon <hidden>
Date: 2018-08-03 15:02:06

03/08/2018 16:16, Parthasarathy, JananeeX M:
Hi Thomas,

From: Pattan, Reshma
quoted
Hi Thomas,

From: Thomas Monjalon [mailto:thomas@monjalon.net]
quoted
01/08/2018 00:18, Reshma Pattan:
quoted
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
quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help