1 May
2019
1 May
'19
11:16 a.m.
I haven't got any response about the misery with siptrace module. I
checked its commit history and become to the conclusion that many
commits to master were done without any review whatsoever.
For example, there was this commit:
siptrace: remove unused trace flag
ionutionita92 authored and henningw committed on Mar 19
which obviously badly broke normal use of the module.
Then 10 days later the commit was reverted:
siptrace: add trace_flag after it was removed
ionutionita92 authored and henningw committed 29 days ago
This is not how Kamailio should be developed.
I'm proposing to revert the module back to 5.2 level after which new
commits can be made to it provided that each commit is carefully tested
before deployed.
-- Juha
1 May
1 May
11:51 a.m.
Hello Juha,
I think the commit history might be a bit misleading here. The siptrace
module was reviewed on the github tracker, and the commits that you
quoted below were the result from this review.
This commits were developed in a private branch, and then completely merged.
You find the review history from me and the responses from the developer
(and others) here: https://github.com/kamailio/kamailio/pull/1912
About the actual issue - I had a look to this as well. It seems that the
logic for the duplicate_uri string might need to be changed.
I will have another look and fix it or will refer it to the original
developer.
Cheers,
Henning
Am 01.05.19 um 10:16 schrieb Juha Heinanen:
I haven't got any response about the misery with
siptrace module. I
checked its commit history and become to the conclusion that many
commits to master were done without any review whatsoever.
For example, there was this commit:
siptrace: remove unused trace flag
ionutionita92 authored and henningw committed on Mar 19
which obviously badly broke normal use of the module.
Then 10 days later the commit was reverted:
siptrace: add trace_flag after it was removed
ionutionita92 authored and henningw committed 29 days ago
This is not how Kamailio should be developed.
I'm proposing to revert the module back to 5.2 level after which new
commits can be made to it provided that each commit is carefully tested
before deployed.
-- Juha
_______________________________________________
Kamailio (SER) - Development Mailing List
sr-dev(a)lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
--
Henning Westerholt - https://skalatan.de/blog/
Kamailio services - https://skalatan.de/services
12:01 p.m.
Henning Westerholt writes:
About the actual issue - I had a look to this as well.
It seems that the
logic for the duplicate_uri string might need to be changed.
I will have another look and fix it or will refer it to the original
developer.
I'm getting en error message to syslog from every request that my sip
proxy is handling. This is pretty serious and should have been noticed
if there was any real review of the commits.
I'm also worried about possible performance implications of these
commits. There should be none compared to 5.2 usage of sip_trace()
function call,
-- Juha
2:07 p.m.
On 01.05.19 11:01, Juha Heinanen wrote:
Henning Westerholt writes:
I don't think it is the case to throw statements like "... if there was
any real review of the commits ...", or how you did it in the previous
message where you wrote:
"I checked its commit history and become to the conclusion that many
commits to master were done without any review whatsoever."
It looks like you try to blame only the others not doing "proper" work
from your point of view, but you didn't even see that those commits were
part of a pull request that was on github from March 28 to April 9, with
couple of people commenting there, with many email notifications sent to
sr-dev.
If a module is important for you, you should keep an eye on it and
participate to the review process. Or, if you missed that time frame
from pull request start to merge moment for what so ever reason, you can
still review and improve -- this is the master branch and till the next
release it should be tuned by those that have an interest in the module.
We all collaborate here for mutual interest and it is important to keep
a decent language and an adequate approach between us. Even if there are
mistakes, they can be fixed without rising the tone or blaming around.
About the actual issue - I had a look to this as
well. It seems that the
logic for the duplicate_uri string might need to be changed.
I will have another look and fix it or will refer it to the original
developer.
I'm getting en error message to syslog from every request that my
sip
proxy is handling. This is pretty serious and should have been noticed
if there was any real review of the commits.
I'm also worried about possible performance implications of these
commits. There should be none compared to 5.2 usage of sip_trace()
function call,
This can be tested, there are ways to do it either via benchmark module
or with the global parameters related to the latency of the actions.
Cheers,
Daniel
--
Daniel-Constantin Mierla -- www.asipto.com
www.twitter.com/miconda -- www.linkedin.com/in/miconda
Kamailio World Conference - May 6-8, 2019 -- www.kamailioworld.com
2032
days inactive
2032
days old
3 comments
3 participants
participants (3)
-
Daniel-Constantin Mierla -
Henning Westerholt -
Juha Heinanen