search

zendframework / zend-servicemanager

ServiceManager component from Zend Framework
BSD 3-Clause "New" or "Revised" License
188 stars 89 forks source link

Request for discussion: Priority of merges to master #246

Closed fhein closed 6 years ago

fhein commented 6 years ago

Currently, bug fixes are merged to master early. There are bugs, but as long as nobody suffers and complains, IMO to fix them does not need that high priority. Those bugs in most cases do not even come to play, because that would require an edge case configuration. Those are rare, I think.

Several people including me think, that one of the most important things for a ground laying component like ServiceManager is performance. That important, that bad performance is seen as a bug. So enhancements here will often get recognized as a bug fix, IMO.

I suggest to rethink, what changes qualify for a quick deployment to master. Would do you think about that?

Ocramius commented 6 years ago

Bug-fixing only on master, everything else goes to develop, no exclusions.

Improvements, whilst welcome, do always go to minor/major as per SemVer, which we adhere as closely as possible.

On 1 Feb 2018 04:18, "Frank Hein" notifications@github.com wrote:

Currently, bug fixes are merged to master early. There are bugs, but as long as nobody suffers and complains, IMO to fix them does not need that high priority. Those bugs in most cases do not even come to play, because that would require an edge case configuration. Those are rare, I think.

Several people including me think, that one of the most important things for a ground laying component like ServiceManager is performance. That important, that bad performance is seen as a bug. So enhancements here will often get recognized as a bug fix, IMO.

I suggest to rethink, what changes qualify for a quick deployment to master. Would do you think about that?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/zendframework/zend-servicemanager/issues/246, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJakOTAtEz0y1atmVhKw3RgLG7txQZkks5tQS0AgaJpZM4R0_sr .

weierophinney commented 6 years ago

@fhein Exactly what @Ocramius noted.

Bugfixes are always prioritized, because those are affecting users now. While performance is definitely a priority as well, if the work involved to achieve better performance is (a) ongoing, and/or (b) involves significant refactoring, we consider this feature development. It is only a bug when a bugfix causes degradation of performance.

Please read about semantic versioning, as that document outlines our policy.

fhein commented 6 years ago

Thanks. This versioning scheme is good practice.

I don't think, that there is urgency to apply #242 for the bug fixes, because those bugs seem to be not critical in the field. Otherwise there would be loud complains.

That brought me to question the classification scheme (i.e. not the versioning scheme). There seem to twilight zone, where it is difficult to say, if something should be classified as a bug or as an enhancement.

Imagine, ServiceManager's get() would take 10 minutes to return a service. This would be considered as a bug, I believe. Allthough it could be seen as an enhancement also.

On the other end of the scale is a bugfix for abstract factories defined via members. Allthough this is a bug definitely, it could be treated like an enhancement (or nice to have bugfix), because nobody defines abstract factories via member obviously.

weierophinney commented 6 years ago

@fhein Again, if a performance regression is reported due to previous changes in code within the same minor release, we'd consider a performance bugfix. Otherwise, performance improvements are generally features for minor releases.

Generally speaking, bugfixes are the result of reported unexpected behavior; features and enhancements are improvements that present new behavior. Improved performance is a new feature generally, unless it is a by-product of a bugfix.

I'd argue #242 is in fact a bugfix, and there are examples of pre-configured service managers in many components: specifically, any plugin manager. The bugfix is primarily around the aspect of properly resolving pre-configured service mappings; the performance improvement is a side-effect.

fhein commented 6 years ago

Agreed. In that particular case performance is a side-effect. And in general performance improvements are features (allthough I still believe, that a 10 min get() would be considered to be a bug ;).

I close that now. Thank you! :+1:

10 min get() is wonderland. In particular I had 36 microseconds for setAlias in mind when overriding an alias.