Breaking changes & versioning

[infostreams]

Hi guys,

to cut right to the chase: why were breaking changes in the Plugin architecture introduced in a minor update (1.4 => 1.5)? Is Phile not following something along the Semantic Versioning guidelines?

Specifically, why did I need to update my Plugin to work with a newer (minor) version of Phile, even though nothing substantial (in terms of features) has changed? Just look at all the ugly code I had to introduce.

Obviously, refactoring is important and it's important to have a clean code base, but could these refactorings have been made while keeping the API backwards compatible? Most plugins in existence will probably use 'injectSettings', but from one day to the next, and not even in a major release, the function is deprecated and removed, silently and unnecessarily killing a whole bunch of plugins. Why?

Additionally, I'd like to complain in advance about the scheduled deprecation of a number of other API methods. I've seen things like @deprecated since 1.5, will be removed in 1.6 in quite a few places, and again the question is: why? Why the rush? Cruft is ugly, but removing it can break things. Can we maybe remove the cruft only when we're sure that the majority is not using it anymore, or if they've had plenty time to update their software (i.e. more than 2 months)?

In summary, can we have a discussion about when to introduce breaking changes, and about how quickly we deprecate internal API calls? My proposal is to just use semantic versioning, and keep the internal API backwards-compatible unless there's a major release (a version 2.0, for example).

Best, Ed

[Schlaefer]

Just look at all the ugly code I had to introduce.

If you use $events you don't have to subscribe to events yourself in __construct() anymore. Just remove the __construct method completely.

Most plugins in existence will probably use 'injectSettings'

I bet five bucks you're the only one. Seriously. :)

Nobody should ever use that method. It's a core implementation detail and an oversight that the method wasn't declared final in the first place. Events are the backwards-compatible way to go.

---

From what I see 1.4 and 1.5 should require no or very minor plugin changes. They are not what I consider major breaking releases.

Problems arise because cruft is kept around to long (e.g. Phile\ vs Phile\Core). Cruft attracts more cruft. Cruft costs time. Fixing it once is O.K., working around it every time is not (at least if nobody pays for it). It blows up the code and makes new development more complicated.

Some features are only made possible by changing existing behavior at least slightly. The cost of moving-on and improving is to spend half an hour and update one or two line of code every half a year. Two months is to fast, I agree. But two times a year should be reasonable.

Of course it sucks to touch existing code and I feel your pain. Fundamentally features should be implemented in a backwards-compatbile manner. And they were in 1.4/1.5. Imho. If they can't, then semantic versioning helps indicating that, but doesn't relieve one from the update-task if you want to support the latest version. But versioning wasn't strictly semantic so far, I agree.

That's the alternative? When I started using Phile I naturally downloaded the 0.x stable release, while the developers already moved on considerably in the 1.x branch.

The only way to guarantee consistent behavior is to limit breaking releases at all or find someone who is willing to maintain (add and remove) backwards compatibility in worst case over several releases.

It's difficult. This little sucker alone is a breaking change. Following semver Phile couldn't release a new version without going to 2.0.

---

All that said there's only so much Phile does. The issue list is not endless, and after those changes are done I assume that "shattering refactoring" will settle down considerably.

[james2doyle]

Phile is still a very young project. I feel your pain of updating to a minor version and things break. It can be really frustrating.

I am keeping my entire site in version control. That is the nice thing about Phile. If I updated and something busted, I would roll back. My blog is actually still on something like 1.1. I haven't taken the time to update it. When I do, I will create a new branch and then do all the updating in there, when it's complete, I will merge that branch in.

This type of workflow should be typical practice for any site or application on a framework (new branch -> pull latest -> fix issues -> merge with master).

I know that this suggestion doesn't address the main issue ("minor" version breaking changes), but I would highly recommend using the type of workflow described above on Phile sites.

[infostreams]

Ok, I understand what you're saying, and I do see that I would be one of the very few people using the injectSettings method. I also agree that cruft needs to go, but can we at least widen the timeframe a bit? Tagging a function as deprecated in 1.5 and then removing it in 1.6 is a bit too quick if you ask me. Better give people ample time to fix and adjust, and wait until 1.8 or 2.0 or so. Otherwise you would force plugin authors to do version detection and have two paths of code to achieve the same.

See the events, for example: I fully agree that the new syntax is much nicer, but it doesn't work in 1.4. Since I don't want to drop support for 1.4, I have to either use the old way in 1.5 as well (but I got an exception/fatal error in 1.5 if I did that), or do some sort of version detection and do one thing for 1.4 and another for 1.5.

Wouldn't it be nicer if I can just build for 1.4 and then assume it will keep on working for at least the next year or so? After such a period I would be more comfortable adding "IE8" code or telling people to upgrade.

[Schlaefer]

Talking Code

See the events, for example: I fully agree that the new syntax is much nicer, but it doesn't work in 1.4. Since I don't want to drop support for 1.4, I have to either use the old way in 1.5 as well (but I got an exception/fatal error in 1.5 if I did that)

Btw just use the non-deprecated version and you should be fine in 1.4 and 1.5

public function __construct() {
    \Phile\Core\Event::registerEvent('before_parse_content', $this);
}

The new syntax is fine, it is supposed to work in 1.4 and 1.5 without touching the code. The problem is a special namespace fubar that went unnoticed. At least by me. You can trigger it with other classes too, not just Event. This was not an intended breakage, it's a bug, that – while a little bit pita to patch now and reverse later – is fixable.

Tagging a function as deprecated in 1.5 and then removing it in 1.6 is a bit too quick if you ask me. Better give people ample time to fix and adjust, and wait until 1.8 or 2.0 or so.

Ok, I agree that target was probably to eager. But don't forget we're guessing actual time here. Maybe 1.6 is out next month, maybe in half a year.

Wouldn't it be nicer if I can just build for 1.4 and then assume it will keep on working for at least the next year or so?

Of course. But it's about balancing that out against other factors (mentioned before).

The API-Intermezzo

Phile has no real API, everyone could potentially call everything. And it's hard to do things the "right way" if so much becomes off limit to change. But while you can call everything, Phile's main "plugin-API" are the events or whatever you find accessible in the plugin-class (don't mention the war initializePlugin()).

Those should theoretically provide everything needed by a regular plugin. They present a promise that is easier to keep than a method in Bootstrap, Core or Utility. That's the reason why the $config is passed in the config_loaded event now, the $plugins in the plugins_loaded; or for the new getPluginPath() method. Those should be treaded with care and move slowly.

If we agree we should maybe codify those notions in a Plugin Best Practices wiki page?

Maybe AbstractPlugin should be even richer in (stable) properties and methods? That's imo up for debate.

Traveling Without Moving

One solution could be forking a branch for the next majorish release early (e.g. 2.0.0). In the meantime there's the strictly non-breaking develop branch (1.y.z).

Non-breaking can release as often as desired. Time between breaking releases (becoming the new non-breaking) is at least six months. Yes, I would start with six months. Sounds short, but two times a year isn't that much either. Keep in mind that's the best (or worse) case scenario #ProgrammerDeadlines :see_no_evil: and that "breaking release" doesn't mean everybody has to rewrite everything. It's in the strict sense of the semver definition.

Legacy code esp. is best declared, maintained and removed by the same person, knowing the what and why. One year is a long time when maintaining OSS. You don't want to leave a mess for the next guy (or gal). Who knows who is fiddling with the code in a year from now. I'm lucky if I can make sense of my own code three months from now, let alone guessing others "don't touch that – yet" intentions. :wink:

[X3msnake]

An i feel @Edward pain, but must agree with @Schlaefer vision.

As much as painfull change can be to all of us, if it is for the better of the many and for the improvement of the project, it should be carried on.

But these changes If anything, should makes us be more alert and pay close attention to the main project, be more involved in the decisions, and be better.

@James Doyle Is there a documented way in philes wiki on the lines to use the roll back to a previous version?

[james2doyle]

There isn't a "Phile" way to roll back. I just mean using git to go back to a previous commit On Sat, Apr 25, 2015 at 6:24 PM X3msnake notifications@github.com wrote:

An i feel @Edward pain, but must agree with @Schlaefer vision.

As much as painfull change can be to all of us, if it is for the better of the many and for the improvement of the project, it should be carried on.

But these changes If anything, should makes us be more alert and pay close attention to the main project, be more involved in the decisions, and be better.

@James Doyle Is there a documented way in philes wiki on the lines to use the roll back to a previous version?

— Reply to this email directly or view it on GitHub https://github.com/PhileCMS/Phile/issues/234#issuecomment-96288594.

[infostreams]

Hey! Thanks for the thoughtful replies and sorry for not replying earlier - I was traveling and without proper internet until now. Most of this reply is @Schlaefer.

Regarding the events: If I use your proposed code

public function __construct() {
    \Phile\Core\Event::registerEvent('before_parse_content', $this);
}

I get a Fatal error: Class 'Phile\Core\Event' not found in ... in my 1.4 setup. So no dice, unfortunately. Using \Phile\Event::registerEvent works in 1.4 but does the namespace bug in 1.5. The solution seems to be to remove the use Phile\Core\Event; line from Core.php in the 1.5 release. If I do that, I can use

public function __construct() {
    \Phile\Event::registerEvent('before_parse_content', $this);
}

and it works in both 1.4 and 1.5. But I'm not sure if that's the way to go (it creates a dependency on a deprecated class) so not submitting a pull request ;-) Even though you could have this dependency now and then re-introduce the use Phile\Core\Event; line when the deprecated class is removed from the code. Anyhow, that's slightly off-topic. [oh, I now see that this is exactly what you do in your pull request. Great!]

For the rest I think I agree with almost everything you say in your reply.

I think your plugin API remarks are spot on. The initializePlugin method seems to me to be mostly internal bookkeeping that regular plugins should not never need to override. Could/should it be moved to __construct, and then have a regular, overridable init() method that every plugin can override if necessary? Just a thought.

Documenting some codified best practices for plugin development sounds like a really good plan.

Regarding your versioning plan: I like it. Adopt as is, I'd say. It would mean that any breaking changes (such as declaring initializePlugin a 'final' method) would have to only be made on this separate branch. Some things would have to be backported from one branch to the other, and this might be a pain. However, I do think that having an upgrade path that just gives you the new features and speed improvements with the promise that your site keeps on working and nothing will break is actually a very nice thing to have. There's a certain mental expectation that 1.4 is probably not too different from 1.5, and when things then do break it's an unexpected annoyance that probably wouldn't have existed if the new version was called 2.0.

These breaking changes should probably be recorded somewhere to go with the release notes. I think the six month period you propose is plenty, not too short and not too long.

So... thanks for the feedback! I think what you proposed is sensible and I hope that these changes get adopted.

All the best, Ed

[Schlaefer]

@infostreams Thanks for the feedback. Please express a simple :+1:/:-1: (+1/-1) on an individual pull-request to indicate approval/disapproval. Feedback on issues/PR moves them forward (faster).

On the point of introducing a new major branch I hope we receive more feedback.

[infostreams]

Thanks, I think my original concerns here have been addressed. I just :+1: 'd your proposed pull requests.

[Schlaefer]

I opened a 2.0 branch for the next breaking release.