Re: [boost] "peer reviewed" - Rights and responsibilities of maintainers

Subject: Re: [boost] "peer reviewed" - Rights and responsibilities of maintainers
From: Alexander Grund (alexander.grund_at_[hidden])
Date: 2018-10-16 15:15:43

> In short, yes. I understand your frustration, but the procedure is to 
> square it off with the maintainer, then if you feel you're getting 
> nowhere, escalate to the list. Basically what you've done, except that 
> you're trying to discuss not the issue at hand, but "rights and 
> responsibilities of maintainers", which is not going to help your 
> issue, or anyone.
I already escalated the issue to the list to avoid getting the crashing 
version into the last release. This obviously failed.
My post here was after in the discussion of/in PR I got the impression 
that I'm expecting to much of the maintainer: A review of my changes, I 
developed, tested and submitted. Someones else agreed to my point of 
view while others opposed. Hence I wanted clarification on the meta 
issue with a reference which I could bring on. Seems like the facts are 
against me in this case so I (or my changes) are in the mercy of the 
maintainer.

As you mentioned it:
#105 adds more CI
#110 includes #105 and tests that singletons are eventually destructed 
and `is_destroyed` returns true in both use cases of the singleton
#111 Is the condensed version of my MWE that shows the crash. Essence: 
Use 2 shared libraries linked against static boost and each uses a part 
of Boost.Serialization. The crash comes from a destruction order 
problem, that is currently unsolved and could be avoided if 
`is_destroyed` would return true, but it doesn't

But Robert insists that #111 "does not show anything at all. It's ill 
conceived." He does not acknowledge that Boost.Serialization makes the 
application crash and does not bring any argument why it would be "ill 
conceived". I do not understand this. It obviously crashes. This is a 
fact easily reproducible by anyone on linux. If the test misuses C++ 
(UB, ...) or the library, then where and how? If not, why is it seen as 
"ill conceived"?

 > My version of the patch captures the essence of the PR while 
retaining the original intent of the code.

This is simply wrong. If it would #110 and #111 would pass. It fails to 
fix the `is_destroyed` function or the core problem with destruction order

 > It's much simpler and alters many less files.

My fix alters 1 file to fix `is_destroyed` and 4 more to remove a single 
line with an assertion which doesn't hold as shown in #111. Most of the 
changes are added reverts to an earlier, working version and comments on 
why and how stuff works, so it doesn't get accidentally broken by 
optimization/reduction efforts as happened in 1.65

 > I have no idea why I am being criticized for making this patch.

Because a) it combines unrelated changes, hiding the fix and b) pushing 
it w/o review by the original patch author who might have reasonable 
arguments why it isn't enough or "the essence of the PR". I learned it 
is your right to do that. But then again it is my right to to tell you 
that your claim is wrong. As I as the author of the patch might 
understand the patch better, it is quite possible, that I'm right, isn't it?