Re: [boost] [Review] Boost.Endian mini-review

Subject: Re: [boost] [Review] Boost.Endian mini-review
From: Beman Dawes (bdawes_at_[hidden])
Date: 2015-02-09 19:39:11

On Thu, Jan 29, 2015 at 1:53 PM, Rob Stewart <rob.stewart_at_[hidden]> wrote:
> On January 23, 2015 11:17:25 AM EST, Joel FALCOU <joel.falcou_at_[hidden]> wrote:
>> The mini-review of Boost.Endian starts today and runs through Sunday,
>> February 1.
>>
>> The docs for the library can be viewed online at
>> https://boostorg.github.io/endian/
>
> I found a few issues on the docs:
>
> * "an class"

Fixed.

> * "may improved efficiency"

Fixed.

> * " Remarks: Meet the EndianReversible requirements", for endian_reverse() should, I assume, refer to "x"

Fixed.

> * should there be a note suggesting that the following be constexpr, when possible?
>
> template <class EndianReversible> EndianReversible conditional_reverse(EndianReversible x, order order1, order order2) noexcept;

I'm leery of that. The user would not be able to depend on a given
function being constexpr because it would change according to the
endianness of the platform. I could be wrong about that, and I'm not
sure if users would get in trouble even if it were correct. The C++
committee is also considering similar scenarios for constexpr, and the
point is far from settled, even between people who understand
constexpr better than I do.

>
> * The effects documented for endian_reverse_inplace() don't allow for specialization.

I'll deal with that separately in a day or two.

>
>  The code type is much smaller than the surrounding text. This is particularly odd, besides annoying, on the specifications section in which the effects, remarks, etc. Are huge compared to the function declaration to which they relate.

I'll try to remove the font-size from the css, and see if that helps.
I currently have it set at 85% because that looks good on my
development machine under Firefox.

I checked firefox, chrome, safari, and IE, using Windows, Linux, OS X,
and Android. While a few of those combinations produced
<code>...</code> a bit on the small side, it was never "much smaller".

It will be tomorrow before I push these changes up to GitHub.

Thanks,

--Beman