BIND 10 #3107: [kean] ./configure report versions of dependencies

[BIND 10 Development]

#3107: [kean] ./configure report versions of dependencies
-------------------------------------+-------------------------------------
            Reporter:  jreed         |                        Owner:  kean
                Type:  task          |                       Status:
            Priority:  medium        |  reviewing
           Component:  build system  |                    Milestone:
            Keywords:                |  Sprint-20130917
           Sensitive:  0             |                   Resolution:
         Sub-Project:  Core          |                 CVSS Scoring:
Estimated Difficulty:  5             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by muks):

 * owner:  muks => kean


Comment:

 Hi Kean

 Adding to what Jeremy said above, here are further review comments:

 * Make a note not to bunch unrelated changes in one commit like
   this. Have one commit per independent change. For example the compiler
   version detection updates would be a single commit, the sed detection
   another commit, etc. It's not elegant to have everything in one
   commit, and reviews become unnecessarily difficult.

 * The branch still hasn't been fixed for every invocation of CPP that
   you have added. Please check and do this change for all cases.

 * The following cannot work:
 {{{
 +CXX_VERSION=`$CXX-V 2> /dev/null | head -1`
 }}}

 This implies it was not tested.

 You should test every single part of your code before putting it to
 review.  You can put the branch to the builders and look at output if
 you don't have direct access to these environments. I've pointed out a
 single issue with non-standard dependency locations 3 times now. Test
 your code before putting it to review. We should not assume that any
 command automatically works. We'll end up with bugs exactly like above.

 Add it to your checklist of things to do before putting something to
 review. You'll get into the habit of doing this.


 Replying to [comment:13 muks]:
 > > As for using angle brackets, the coding guidelines apply to actual
 > > source code, not to configure scripts, surely? There is good reason
 > > to use quotes in configure scripts, where the ordering of -I options
 > > is far more critical during feature detection.
 >
 > Please state in unambiguous terms with an example what these reasons
 > are at the place where this test program is processed (after log4cplus
 > INCLUDES and LIBS are determined), and why you'd want one style of
 > `#include` in `configure.ac` and another in C++ source code in this
 > case.

 You chose an approach against the coding guidelines and stated that you
 picked it for some reasons. Now you have used the style in the coding
 guidelines without explanation. This isn't convincing either way and
 leaves me confused. Can you explain what was asked for above? What is
 the reason why you argued against using the style in the coding
 guidelines? We can pick the better approach, and also modify the coding
 guidelines if necessary.

 The `#include` style is also not fixed everywhere. Please check the
 additions thoroughly. I have not continued with a complete check.  If
 you are on a branch currently and want to do a diff against `master` to
 see all the changes, you can run:

 {{{
 git diff master... # with 3 dots
 }}}

-- 
Ticket URL: <http://bind10.isc.org/ticket/3107#comment:17>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development