[Asterisk-code-review] BuildSystem: Go for Python X.Y explicitly. (asterisk[master])

[Matthew Fredrickson]

Matthew Fredrickson has posted comments on this change. ( https://gerrit.asterisk.org/8673 )

Change subject: BuildSystem: Go for Python X.Y explicitly.
......................................................................


Patch Set 2: Code-Review-1

> Glad you got it finally. However, the behavior is unacceptable. The
 > reason for your -1 got void. In such a case, you remove that -1. If
 > you have a new or different concern, you add that with a new -1. I
 > cannot see in your mind and know whether you got it or whether you
 > have some different reason. I was stranded, because I did not know
 > how to proceed.
 > 
 > OK, you gave another -1: ‘Please, wait for the other change’.
 > Again, this is not acceptable behavior, because I am punished for
 > revealing all this stuff: I have to argue, convince, wait, and then
 > rebase my change because it revealed something else. I am unhappy
 > about that rebase in particular because that really takes my time.
 > By the way, I have to create a complete new commit message. You are
 > blocking mine previous legitimate change. Furthermore, instead of
 > getting a ‘thank you’ or at least ‘please’, you order me to go for
 > your suggestion like being a subordinate of yours.
 > 
 > Consequently, I have to align with a new situation and again find a
 > solution to get this issue solved finally. This is punishment of
 > external contributors and unacceptable behavior in a code review. I
 > love to help and sometimes one unveils bigger things. OK. However,
 > the one doing all this should not be placed even higher burdens on
 > his shoulder. If you punish the one who reveals something, he is
 > not doing this again.

Hey Alex,

I don’t think that anything Corey was saying was meant to be malicious in nature.  As developers, respectful language is important, but I think as developers in a peer review process we try to be technical first and sometimes forget to be personable as well.  I didn’t see anything overtly unkind on the comments of this review, so perhaps there’s maybe a language barrier mismatch in terms of gerrit/text communication.
 
As I’m sure you know, just because we contribute a patch, it doesn’t necessarily mean that it will automatically be included.

Corey had at least one request you didn’t address originally - that is your choice as someone contributing code, but in a peer reviewed code contribution process, the purpose is to make sure that you have technical buy-in from other interested and vested parties that may be reviewing your code before the code is merged.

After discussing this with Corey, I think the preference he and I landed on for the configure.ac changes is:

AC_PATH_PROGS([PYTHON], [python python3 python2 python3.6 python2.7], :)

and

AST_PKG_CONFIG_CHECK([PYTHONDEV], [python < 3])
AST_PKG_CONFIG_CHECK([PYTHONDEV], [python2])
AST_PKG_CONFIG_CHECK([PYTHONDEV], [python-2.7])

Alexander, as a project, we appreciate your contributions - you are one of the more regular contributors to the project and I think that everybody is grateful for that.  Part of the responsibility of working on an open source project with lots of stakeholders is to make sure your patch passes the review process - that is what keeps the code clean and how we try to catch bugs or problems before they are included.  That can be a frustrating process when you’d like to get your patch in quickly, but in order for this particular patch to move forward, the suggested changes need to be included (unless there’s something specifically wrong with them).

Best wishes,
Matthew Fredrickson


-- 
To view, visit https://gerrit.asterisk.org/8673
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I29e694dc7e92510dc26b15895b55f78f67146b2c
Gerrit-Change-Number: 8673
Gerrit-PatchSet: 2
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Comment-Date: Fri, 20 Apr 2018 20:26:04 +0000
Gerrit-HasComments: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180420/f2f6556f/attachment-0001.html>