Re: [boost] [units] Pull request and RFC: information unit definitions for Boost.Units

Subject: Re: [boost] [units] Pull request and RFC: information unit definitions for Boost.Units
From: Jürgen Hunold (jhunold_at_[hidden])
Date: 2014-08-27 10:56:30

Hi Gonzalo,

Am Mittwoch, 27. August 2014, 12:49:49 schrieb Gonzalo BG:
> On Wed, Aug 27, 2014 at 12:21 PM, Jürgen Hunold <jhunold_at_gmx.eu> wrote:
> > - Documentation:
> > - (Examples)
> 
> I'll add a small section to the documentation with an constexpr example.
> The (automatically generated?) API reference documentation already includes
> the BOOST_CONSTEXPR in the data-types member functions which should be
> self-explanatory.

Yes, doxygen driven

> I'll ping you when I update the PR and we can discuss
> there how to improve the documentation further but basically either you can
> use Boost.Units in constant expressions or you can't.

Sounds reasonably

> > - Tests
> 
> I have some unit-tests but removed them because I wasn't able to enable
> them only for compilers with constexpr support (otherwise they will fail to
> compile). 

Great news. 

> Boost.Build/Boost.Test documentation wasn't very helpful in this
> regard. 

Unfortunately. Just get the test to work with compliant compilers and I'll 
have a look. 

> I just went through Boost.Array develop branch and found:
> 
> https://github.com/boostorg/array/blob/develop/test/array_constexpr.cpp
> 
> Should I implement the constexpr tests in a similar manner or do anything
> differently?

No, this is a valid approach for me. Complicating the Boost.Build logic for 
the tests is not necessary.

> > And I'd like a snappy sentence to add to the release notes.
> 
> I'll add "Boost.Units has gained constexpr support." to the commit message.

Thanks. 

> > - The patch also changes unrelated whitespace like line-endings, removes
> 
> empty
> 
> > lines and changes wrapping. This should go into a separate pull request if
> > necessary.
> > - The patch also removes some code blocks currently commented out. This
> 
> should
> 
> > be done in a separate request, too.
> 
> I was just following the boy scout rule but will revert them.

Well, I try to separate the style changes while editing the commits. This one 
of the reasons I really like git(k) :-) Following the boy scout rule is fine 
otherwise.

> I won't create a new pull-request with these but I since navigating the
> code in a 80char terminal was a bit unpleasant I would be happy if you
> consider some of them in the future.

Well, 80 chars is quite small, I'd raise the bar to 120 or so myself.  I think 
I'll revisit the changes when reviewing your updated PR.

Yours,

Jürgen

-- 
* Dipl.-Math. Jürgen Hunold  ! 
* voice: ++49 4257 300       ! Fährstraße 1
* fax  : ++49 4257 300       ! 31609 Balge/Sebbenhausen
* jhunold_at_gmx.eu             ! Germany