I think we can make your changes Gilles, if we do a 2.0. Would you prefer to go that route? If we go with 1.1 (other than via deprecation), I think we’re stuck behind a bunch of -1 votes, and Id rather not roll an RC to that audience. I can work on the BC changes or the version changes between now and next week.
-Rob > On Aug 9, 2018, at 10:49 AM, Gary Gregory <garydgreg...@gmail.com> wrote: > >> On Thu, Aug 9, 2018 at 8:47 AM Gary Gregory <garydgreg...@gmail.com> wrote: >> >> >> >>> On Thu, Aug 9, 2018 at 7:25 AM Rob Tompkins <chtom...@gmail.com> wrote: >>> >>> >>> >>>> On Aug 9, 2018, at 9:17 AM, Gilles <gil...@harfang.homelinux.org> >>> wrote: >>>> >>>> On Thu, 9 Aug 2018 08:43:34 -0400, Rob Tompkins wrote: >>>>>> On Aug 9, 2018, at 8:38 AM, Gilles <gil...@harfang.homelinux.org> >>> wrote: >>>>>> >>>>>>> On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote: >>>>>>> Are we instead trying to remove the “extends” from all of those >>> classes? >>>>>> >>>>>> I'm not sure what you mean, sorry. >>>>>> >>>>>> "SamplerBase" was meant has a utility/shortcut/boiler-plate in order >>>>>> to copy the "rng" argument in one place, instead of having a field >>>>>> in each sampler class. It is purely internal to this library (in C++, >>>>>> inheritance would have been "private”). >>>>> >>>>> Are we trying to remove SamplerBase all together? >>>> >>>> That would break BC the same way. >>>> I wouldn't mind since the shortcut is not really significant. >>>> >>>> But my reasoning would be the same: correct usage does not refer >>>> to these methods so the fix could be done now, with minimal risk >>>> (due to low usage of a new component). >>>> >>>>>>> My thought was to override the method and javadoc locally (in the >>>>>>> actual subclass) so that we can give reasonable deprecation messages, >>>>>>> but that’s only a thought. I’m ok with whatever. >>>> >>>> It's overkill in this context. >>>> We shouldn't be rigid about the "no BC in minor release" rule; >>> >>> I’m a tad confused here. I thought that the whole point of everything >>> that we do in terms of the way we version and name packages >>> was specifically driven at the fact that we’re completely rigid about “no >>> BC breakage in minor releases.” >>> >>> I’ve not been around the project long enough to know this with certainty, >>> but that definitely is the vibe that I get. I would think that such RCs >>> would generally get -1’s from people. >>> >>> Generally I’m open to whatever. I’m just operating on what I perceive to >>> be precedent. >>> >>> @Gary - what’s the call here on the flexibility of BC incompatible >>> changes in a minor release? >>> >> >> BC is binary and we should not break stacks. You really don't want to >> create jar hell for our users. Think about deep transitive dependencies. An >> end use or developer is not going to care 2 cents about "correct vs >> incorrect API usage" when a component depends on this depends on that and >> so on, ends up breaking his or her whole build because a low level >> component has decided that breaking BC was OK. >> > > I'll also note that I am not close enough to this component to opine on the > quality of the API or the workarounds discussed. My point is only "Don't > create jar hell; don't break BC." > > Gary > > > >> >> Gary >> >> >>>> enforcing it is worse for everybody: >>>> * bona fide users will be forced to modify their source and >>>> recompile in order to benefit from the performance boost >>>> introduced in version 1.1, and >>>> * bad usage (if it exists at all) could continue unsuspected. >>>> >>>>>> The message would aim at the wrong target: for developers of this >>>>>> library, there is no deprecation (the boiler-plate code is there >>>>>> to be used); for application developers, the class should not be >>>>>> there to be used (hence: package private). >>>>> >>>>> Then, I guess, we should simply state that we want to eventually >>>>> delete these methods and they should not be consumed. Yeah? >>>> >>>> No (cf. above); I propose to delete them *now*. >>>> >>>>> Like I >>>>> said…I’m up for whatever. I’m just trying to balance yours and Gary’s >>>>> points and find a good middle ground that everyone can live with. >>>> >>>> Which are the two sides for which you try to find a middle ground? >>>> At any rate it would not be "good" if it makes developers and users >>>> unhappy for a case that probably doesn't exist out there. >>>> Tools such as Clirr are a great help, but they shouldn't induce us >>>> to take the wrong course only to have the pleasure to contemplate a >>>> clean report. ;-) >>>> Thanks to Clirr (and Gary), we've become aware of a design issue. >>>> We can decide to resolve it as it was done in the past, with IMHO >>>> zero inconvenience (except for the Clirr "unclean" report). >>>> >>>> Regards, >>>> Gilles >>>> >>>>> -Rob >>>>> >>>>>> >>>>>> Gilles >>>>>> >>>>>>> -Rob >>>>>>> >>>>>>>> On Aug 9, 2018, at 7:02 AM, Gilles <gil...@harfang.homelinux.org> >>> wrote: >>>>>>>> >>>>>>>> Hi all. >>>>>>>> >>>>>>>>> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote: >>>>>>>>> Hello Rob. >>>>>>>>> >>>>>>>>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote: >>>>>>>>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m >>>>>>>>>> only a +0 on this change. So, if you want to revert it before >>> going up >>>>>>>>>> with 1.1, that’s fine. >>>>>>>>> >>>>>>>>> I don't understand this change after what I answered to Gary's >>> strange >>>>>>>>> proposal. >>>>>>>>> The comment is *wrong*. As I said previously, the base class >>> provides >>>>>>>>> boiler-plate code; that is *not* deprecated. The issue is that >>> those >>>>>>>>> methods were not meant to be used further down the hierarchy (in >>> user's >>>>>>>>> subclasses). [And now, furthermore, they are not used anymore in >>> class >>>>>>>>> "PoissonSampler", following the change in RNG-50 (delegation to >>> other >>>>>>>>> classes).] >>>>>>>>> The sampler classes should have been made "final"; but now, this >>> change >>>>>>>>> would also be "breaking" (even though I doubt about legitimate >>> use-cases >>>>>>>>> for inherithing from the sampler implementations). >>>>>>>> >>>>>>>> Assuming it's unlikely that application developers would have >>>>>>>> * called protected methods of "SamplerBase",[1] >>>>>>>> * create subclasses of "SamplerBase",[1] >>>>>>>> * create subclasses of the sampler classes,[1] >>>>>>>> I suggest to >>>>>>>> * make "SamplerBase" package private >>>>>>>> * make the sampler clases "final". >>>>>>>> >>>>>>>> The above are the *less* intrusive fixes, as they would only >>>>>>>> potentially impact application developers by making them aware >>>>>>>> that they have made incorrect usage of an *inadvertently* >>>>>>>> public API. Correct usage will not be impacted even though >>>>>>>> the change is not BC.[2] >>>>>>>> >>>>>>>> Any objection to that fix?[3] >>>>>>>> >>>>>>>> Regards, >>>>>>>> Gilles >>>>>>>> >>>>>>>> [1] Rationale: (a) There is no reason to do that and >>>>>>>> (b) "Commons RNG" isn't much used at this point: >>>>>>>> >>> http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages >>>>>>>> [2] I recall that a similar situation (BC breaking in a minor >>>>>>>> release) occurred in CM, at a time where the number of >>>>>>>> potential users was much larger. >>>>>>>> [3] I'll add a prominent warning in the changelog to the effect >>>>>>>> that people wanting to continue with incorrect usage of the >>>>>>>> samplers should not upgrade. ;-) >>>>>>>> >>>>>>>>> >>>>>>>>> Please revert. >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Gilles >>>>>>>>> >>>>>>>>>> >>>>>>>>>> -Rob >>>>>>>>>> >>>>>>>>>>> On Aug 8, 2018, at 12:42 PM, chtom...@apache.org wrote: >>>>>>>>>>> >>>>>>>>>>> Repository: commons-rng >>>>>>>>>>> Updated Branches: >>>>>>>>>>> refs/heads/1.1 50b984b1d -> f8159f28a >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Adding PoissonSampler deprecations. Use the correct faster >>> public methods >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo >>>>>>>>>>> Commit: >>> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28 >>>>>>>>>>> Tree: >>> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28 >>>>>>>>>>> Diff: >>> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28 >>>>>>>>>>> >>>>>>>>>>> Branch: refs/heads/1.1 >>>>>>>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0 >>>>>>>>>>> Parents: 50b984b >>>>>>>>>>> Author: Rob Tompkins <chtom...@gmail.com> >>>>>>>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400 >>>>>>>>>>> Committer: Rob Tompkins <chtom...@gmail.com> >>>>>>>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400 >>>>>>>>>>> >>>>>>>>>>> >>> ---------------------------------------------------------------------- >>>>>>>>>>> .../sampling/distribution/PoissonSampler.java | 42 >>> ++++++++++++++++++++ >>>>>>>>>>> 1 file changed, 42 insertions(+) >>>>>>>>>>> >>> ---------------------------------------------------------------------- >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java >>>>>>>>>>> >>> ---------------------------------------------------------------------- >>>>>>>>>>> diff --git >>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java >>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java >>>>>>>>>>> index d0733ba..29d0e4e 100644 >>>>>>>>>>> --- >>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java >>>>>>>>>>> +++ >>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java >>>>>>>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler >>>>>>>>>>> return poissonSampler.sample(); >>>>>>>>>>> } >>>>>>>>>>> >>>>>>>>>>> + /** >>>>>>>>>>> + * @return a random value from a uniform distribution in the >>>>>>>>>>> + * interval {@code [0, 1)}. >>>>>>>>>>> + * @deprecated - one should be using the {@link >>> PoissonSampler#sample()} method, >>>>>>>>>>> + * as it is considerably faster. >>>>>>>>>>> + */ >>>>>>>>>>> + @Deprecated >>>>>>>>>>> + protected double nextDouble() { >>>>>>>>>>> + return super.nextDouble(); >>>>>>>>>>> + } >>>>>>>>>>> + >>>>>>>>>>> + /** >>>>>>>>>>> + * @return a random {@code int} value. >>>>>>>>>>> + * @deprecated - one should be using the {@link >>> PoissonSampler#sample()} method, >>>>>>>>>>> + * as it is considerably faster. >>>>>>>>>>> + */ >>>>>>>>>>> + @Deprecated >>>>>>>>>>> + protected int nextInt() { >>>>>>>>>>> + return super.nextInt(); >>>>>>>>>>> + } >>>>>>>>>>> + >>>>>>>>>>> + /** >>>>>>>>>>> + * @param max Upper bound (excluded). >>>>>>>>>>> + * @return a random {@code int} value in the interval >>> {@code [0, max)}. >>>>>>>>>>> + * @deprecated - one should be using the {@link >>> PoissonSampler#sample()} method, >>>>>>>>>>> + * * as it is considerably faster. >>>>>>>>>>> + */ >>>>>>>>>>> + @Deprecated >>>>>>>>>>> + protected int nextInt(int max) { >>>>>>>>>>> + return super.nextInt(max); >>>>>>>>>>> + } >>>>>>>>>>> + >>>>>>>>>>> + /** >>>>>>>>>>> + * @return a random {@code long} value. >>>>>>>>>>> + * @deprecated - one should be using the {@link >>> PoissonSampler#sample()} method, >>>>>>>>>>> + * * as it is considerably faster. >>>>>>>>>>> + */ >>>>>>>>>>> + @Deprecated >>>>>>>>>>> + protected long nextLong() { >>>>>>>>>>> + return super.nextLong(); >>>>>>>>>>> + } >>>>>>>>>>> + >>>>>>>>>>> /** {@inheritDoc} */ >>>>>>>>>>> @Override >>>>>>>>>>> public String toString() { >>>>>>>>>>> >>>> >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>> >>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>> For additional commands, e-mail: dev-h...@commons.apache.org >>> >>> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
