emogrifier
emogrifier copied to clipboard
MyIntervals
@internal instead of private?
Just had this thought which is relevant to #668.
If we made the methods protected instead of private, but also marked them as @internal, wouldn't that allow those who wanted to hack around with the classes a bit to do so more easily, whilst also saying that we provide no warranty that any new version (even a patch) would be backward-compatible, thus it would be up to them to carefully test any new version if they override @internal methods, but we allow them to do so if they so choose, at no cost to ourselves?
Best of both worlds?
WDYT @all?
I disagree. I think that it's important that we provide a clear contract, i.e.:
- public methods = a promise that those won't change except with a new major release
- protected methods= a promise that those won't change except with a new major release
- public methods marked
@internal= no promise - protected methods marked
@internal= no promise
If someone needs some method to be part of the API, they can open a ticket, and then we can decide whether to do this.
I think that this will make both us as well as the users more happy in the long run.
- protected methods marked
@internal= no promise
That's my point - there is no promise - use at your own risk. Though I can see a potential for headaches if someone has overlooked the @internal in the DocBlock.
I was referring to people needed to call a currently-private method. In that case, I'd prefer us to make it protected/public without @internal, i.e., make it API with a promise.
I'm just thinking of the general case. #668 was just an example. I don't think any methods should be made part of the API without a valid reason. Though I suppose if they were all made @internal protected and there was a valid reason to make one of them part of the API, we may not get to find out (until we change it in some compatibility-breaking way), because people may start using it without telling us.
I think we might want to go through our current public and protected methods and check which are not part of our current public API and should be marked as @internal, though, in order to avoid unintentional fallout when we change something there.
WDYT?
Not a bad idea. I'd be willing to bet that there aren't any, but wouldn't offer better than even money, as it's quite possible we may have missed something. I'm aware of several classes marked @internal which I assume suffices for the entire class.
I'm aware of several classes marked
@internalwhich I assume suffices for the entire class.
Yes, that's the way I understand (and intended) this as well.
On the OP, I think we are unlikely to reach consensus between the two of us, and on that basis we should do nothing. But before closing this, I'd like to leave it a while to see if @zoliszabo or @jjriv have anything to say.
I'm not as involved with this project so take my opinion with a grain of salt :) I would leave things as is, unless there are known use cases or there have been requests to change these methods to protected. But even if there were, I would encourage the developer to submit the change as a PR so that it can go through a more formal review process.
I would leave things as is, unless there are known use cases or there have been requests to change these methods to protected.
#103 is the only other that I'm aware of, besides #668 for which an alternative solution was proposed anyway (though, to this day, never implmented).
My gut feeling is still "if it aint really broke, and there is no consensus, don't fix it", although my personal opinion is preferring methods never being private and classes never final - I have had to hack so many third party libraries over the years that have deliberately not allowed extensibility in this way, but which had shortcomings that would never be resolved on the timescale I needed (if even at all).