[Asterisk-code-review] build-system: Allow building with static pjproject (asterisk[13])

[George Joseph]

George Joseph has posted comments on this change.

Change subject: build-system: Allow building with static pjproject
......................................................................


Patch Set 24:

> As far as the build system stuff is concerned, you get an implicit
 > +1 from me because my brain goes numb when I start looking at that
 > stuff.
 > 
 > What concerns me here is the library of patches. I'm fine with
 > maintaining this sort of directory as long as the patches are
 > 
 > 1) Also contributed upstream to PJProject
 > 2) Directly fixing a problem that has been reported in Asterisk.

This was always the plan.

 > 
 > Some of the patches on this issue appear to conform to what I
 > suggested. Others, I'm not so sure about (e.g. log level patch, no
 > third party patch).

The log level patch was submitted and merged into pjproject master.

The no-third-party one is a patch that Fedora applies and I had it in there to start.  I'll remove it as it doesn't really apply.

 > I would suggest that any patch that does not
 > conform to the above guidelines should not be included in the
 > Asterisk source. My main concerns are that
 > 1) We may be tempted to go the easy way with certain bug fixes and
 > write a patch for the patch library instead of doing the right
 > thing and contributing upstream.

Agree 100%.  Except for the errant third-party patch, they were all submitted and accepted.

 > 2) The maintenance task of keeping patches up-to-date when new
 > versions of PJProject are released can snowball more quickly than
 > we would like. Using only patches contributed upstream means we
 > just empty the patch directory with each new PJProject release.

Exactly.

I would say though that there *may* be times a change would be beneficial for Asterisk but would conflict with pjproject's mission of providing a library for client applications.  We shouldn't automatically reject a patch like this but the justification and documentation level required should be fairly high.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia7a60c28c2e9ba9537c5570f933c1ebcb20a3103
Gerrit-PatchSet: 24
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: No