Liipimaginebundle: Breaking Changes / Road-map v2

[robfrawley]

Road-map 2.0

_Note: this will serve as an alternate, more ambitious (by which I mean, no regard for BC breaks whatsoever with an eye toward the future and disregard for the past), alternative to #686._

• [ ] Code cleanup (consider ways to remove duplication and otherwise refactor code for the better)
• [ ] Symfony 2.8+ (with _no usage of deprecated features, full 3.0+ compatibility_)
• [ ] PHP 5.6+ (or maybe 7.0+)
• [ ] LoaderInterface Rename (rename LoaderInterfaces under Liip\ImagineBundle\Imagine\Filter\Loader and Liip\ImagineBundle\Binary\Loader to remove confusion)
• [ ] FilterLoader Rename ("Loader" is confusing in this context, perhaps "FilterProcessor" or "FilterRunner" or similar?)
• [ ] Binary/FileBinary Standardization (Remove ability to return or provide as argument both models in a bunch of code, standardize on one wherever possible.)
• [ ] Move Controller Business Logic into Service (Allow for use of service without Request)
• [ ] Merge ConfigurablePostProcessorInterface and PostProcessorInterface
• [ ] See about supporting usage cases like #733
• [ ] Conversion between formats (ex: PNG -> JPG)

More to be decided... (post additions in the comments; ideas welcome!)

Contributed Additions

@antoligy

• Currently Symfony's minimum version requirement is 5.5.9
• Imagine supports 5.3; Perhaps such an upgrade should be a collaboration effort?
• We gain [many PHP7 benefits due to] [potential new IMagick transforms and features](https://pecl.php.net/package-changelog.php?package=imagick).
• Not the same for GMagick sadly.
• Plenty of bugfixes to GD.
  

  Original Message

@lsmith77 Any interest in creating a 2.x branch where we can start shaping this bundle's next major release? It's time to drop PHP versions that are no longer supported, as well as Symfony versions that have reached EOL. There are some structural changes that could benefit the code base as well.

I'd love to help!

[lsmith77]

I am certainly open to exploring a 2.x.

see also https://github.com/liip/liipimaginebundle/issues?q=is%3Aopen+is%3Aissue+milestone%3Av2.0

[robfrawley]

Any chance of getting a 2.x branch that a larger number of people can directly contribute to? Or should we just keep work on our respective personal forks?

[jehaby]

Yeah, please create the branch for new release. I will be happy to contribute to it.

Also I wanted to suggest considering to drop support of PHP versions below 7.0 altogether in it. I'v seen the roadmap, but I think it would be really nice to be able to use all new PHP features. And it should be good for attracting new contributors.

[robfrawley]

@jehaby My assumption is that the maintainers will likely not agree to dropping all PHP 5.x support in favor of only 7.0 support.

That said, it would be interesting to start to push the envelope with regard to support for PHP 7.0 by maintaining a 2.x and 3.x brand with support for PHP 5.x and PHP 7.x, respectively. IDK. Just a thought.

[lsmith77]

hmm what is the time line we see for this?

I pushed a 1.0 and 2.0 branch because I couldn't make up my mind what "master" should be in the future .. even pondering to drop master entirely ..?

[robfrawley]

@lsmith77 (re: timeframe): That's a good question; I honestly hadn't really given that too much thought. I'd like to get something solid, positioned as an alpha within 6 months (hopefully). I _mainly_ wanted to petition for a new branch so we would be able to start implementing large, breaking changes, and go from there.

(re: branches): No preference what-so-ever. I've seen it handled both ways in different repositories, so whatever method you'd prefer. Removing master _does_ provide clarity as to exactly what each branch _is_.

[alexwilson]

If we're straying from the roadmap, I guess it might make sense to figure out exactly what sorts of changes we would like to see that break BC "1.0" versions - What baseline do we set for cutting off support?

PHP7 brings nice new features, but I would argue that they are non-essential to the upkeep of this codebase.

• What new features would we benefit from in the short term?
• What sorts of contributions would updating the code to use PHP7 syntactic sugar encourage?
• Do we backport features from the 2.0 to 1.0?

[lsmith77]

I am open to going to php 7.0+ but I prefer it to be based on real concrete issues that it solves.

[alexwilson]

Agreed, 7.0 BC breakage shouldn't be done for the sake of it, and if we do go down that route we should make sure there's some kind of stance on support for older versions in place first.

Re. branches, dropping master is an option: personally I think maintaining master and a separate 1.0-x branch might be the way to go, as then we can backport "essential" features, while making it very apparent that it is not the main stream of development.

[robfrawley]

So, this isn't necessarily a stance on the minimum version decided on for _this bundle_, but is more a general-purpose argument for PHP 7.x. And many older PHP coders will likely dislike it, but...

_Scalar type hints, return type declarations, and strict mode._ Many may see scalar type hints and return types as syntactic sugar, but such a feature is much more important that simply providing context for contributors. It allows those that use this library to optionally enable strict mode, as well as allows us to enable strict mode in our test suite, a feature which often aids in catching bugs and other abnormalities in a large code base. This _alone_ is reason enough for me to use PHP 7 in all my upcoming consultation projects.

Again, the value of this as it pertains to this bundle is debatable, but I feel like this often gets overlooked or otherwise not weighted with a high level of value. To me, this feature is invaluable.

If we don't set PHP 7.0 as the minimum, I would strongly argue for PHP 5.6, as this is the only PHP 5.x series release that hasn't reached EOL, and as a community we should do everything we can to force people to move onto supported PHP versions. Also, variadic functions are wonderful, as is argument unpacking.

[alexwilson]

Strict mode isn't the only reason I'd be in favour of going forward with PHP7 (I am already a strong proponent most of the time, so you're preaching to the choir! 😛) - It's the legacy products which use this, some of which I maintain, that concern me when it comes to preserving BC.
With future image optimisation related changes, I would like to be able to bring these back to these other products (and I don't think I'm alone in thinking this). Given the opportunity to get rid of the PHP5ish-era legacy in favour of PHP7, I would, but unfortunately there are time and business constraints which prevent this from happening immediately.

Things to bear in mind:

• Currently Symfony's minimum version requirement is 5.5.9. At time of writing, this is still the case for the upcoming 3.2 (also looks like PHP7 error handling is improved in 3.2).
• With PHP7 we would still not be able to enable strict mode, as Imagine supports 5.3; Perhaps such an upgrade should be a collaboration effort? By the looks of it, there's a slight bug with the imagemagick extension in PHP7 which is preventing the ability to update.
• We actually gain a lot from the update to PHP7 in terms of potential new transforms and features we can perform via IMagick!
• Not the same for GMagick sadly.
• Plenty of bugfixes to GD.

PHP 5.6 sounds like a safe medium, at least for syntax, as the changes between 5.3 and 5.6 are far easier to implement, and I am in favour. I think going forward it might make sense to try and bring the update efforts to the Imagine library, and to revise the original roadmap to bring in PHP7.

[alexwilson]

Oops sorry, looks like you've already proposed a new roadmap! Looks good!

Something to tack on, with the chance to break BC I'd like to make a few alterations to the PostProcessors, some of which I think might warrant discussion. Least of all, I think we should merge ConfigurablePostProcessorInterface with the PostProcessorInterface.

This is part of a grander scheme in which ideally we should discourage preservation of state in either filters or post-processors, however I will raise this under a separate issue (and PRs) if and when this idea is approved.

[robfrawley]

Least of all, I think we should merge ConfigurablePostProcessorInterface with the PostProcessorInterface.

Absolutely.

[alexwilson]

Should we amend contributor guides to talk about the distinction between the two branches?

[robfrawley]

@antoligy IDK if we're quite using them yet: master remains ahead of both 1.x and 2.x. If we do move to such a model, we should update the guides. @lsmith77 did you decide how to proceed with branching?

@lsmith77 Can we enable the public WIKI in this repo's settings, so we can define a more firm roadmap collaboratively?

[robfrawley]

@lsmith77 @jehaby @antoligy

Created src-run/liip-imagine-bundle-sandbox as a place for many, disparate developers to have write access and toy with possible directions for this bundle in a collaborative manner. If anyone wants access, post here and I'll add you to the group I created on my organization for this purpose: https://github.com/orgs/src-run/teams/imagine

Here are the rules:

• Master reflects the official intention of the consensus (this will be where pull request back upstream originate from and is a protected branch that only administrators can write to: at this time only myself though I hope to add others).
• The 2.x branch allows write access to all members of the src-run/imagine group.
• Anyone and everyone is allowed to ask to be added to the src-run/imagine group. Want to collaboratively work in a sandbox with the intention of fleshing out good and bad ideas for upstream submission? Just ask here and you will be!
• Three strikes causes your forever removal from this group. "Stikes" are subjective, but getting one against you will only occur if you are doing something _intentionally destructive_ or _unintentionally harmful_. Basically, if you are careful and actively trying to contribute to the direction the contributor consensus has formed, you won't have an issue.
• PRs from this repo should not necessarily be squashed as they may contain large contributions whith useful Git history.

What do you guys think of this? I think it would be fun and more productive to have a place to work collaboratively without the same quality standards required in the official project. Bad idea? Tell me why. Either way: let's start making a new major release!

Links to services tracking this sandbox:

• https://travis-ci.org/src-run/liip-imagine-bundle-sandbox
• https://coveralls.io/github/src-run/liip-imagine-bundle-sandbox
• https://www.codacy.com/app/src-run/liip-imagine-bundle-sandbox/dashboard
• https://styleci.io/repos/67658886

[lsmith77]

no .. haven't made a decision. my gut feeling says its clearer for contributors to know where to point their PRs to if we drop master and just keep 1.0 (as the default branch) and 2.0 ..

so I propose to drop master .. ok?

[robfrawley]

Sounds good to me. (Deletion of master, that is).

[robfrawley]

@lsmith77 FYI: Right now master is ahead of 1.0 and 2.0

[jehaby]

If anyone wants access, post here

++

[robfrawley]

@jehaby Invitation sent last night, actually. ;-)

[makasim]

There are my thoughts on what could be done in 2.x version: https://github.com/liip/LiipImagineBundle/issues/686

[alexwilson]

Looks good, I believe most of those have already been merged into the revised OP for this Issue, but maybe it's worth going through with a fine-toothed comb?

_e.g. We've agreed that the new minimum PHP version should be 5.6 (despite Symfony still supporting 2.5; this will allow us to improve the reliability of tests, whilst allowing us to backport features with relatively little effort to the 1.0 branch)._

[robfrawley]

@antoligy (think you meant to say "...despite Symfony still supporting 5.5...")

If someone wants to start outlining v2, I've opened up the WIKI in my sandbox repo for all to edit: https://github.com/src-run/liip-imagine-bundle-sandbox/wiki

[robfrawley]

@makasim Yeah, PHP 5.5 for me is a no-go, both from the perspective of enjoying and regularly using features that it doesn't support, but also from the perspective of being a positive force in the community by helping force people off of EOL (end of life) PHP releases. If libraries/etc don't require it, people won't ask their shared host to support it, and people will continue to use EOL releases and upgrades will lag behind. IMHO

[robfrawley]

@lsmith77

We should figure out the branch situation, as it is a bit confusing to look at Packagist at the moment: https://packagist.org/packages/liip/imagine-bundle#dev-master

After some more thought, I don't think we should drop master, as I believe it will make branch aliasing not function correctly (correct me if I'm wrong). Something like this won't work if we only have a 1.x and 2.x branch:

"extra": {
    "branch-alias": {
        "dev-master": "1.6.x-dev"
    }
}

Removing master will make it impossible to use the common "dev-master" alias (even though doing so is generally a bad idea)...

So, I propose:

• Remove 2.x
• Make master 2.x
• Keep 1.x

We can continue to have the GitHub default branch be 1.x for the foreseeable future, as well. The only thing we need to document if we used the above layout, would be that if someone still wanted to get 1.x-based dev in composer, they would need to change "dev-master" to "1.0.x-dev" as "dev-master" would now pull 2.0.

Lastly, we need to make sure the branches are kept in sync! This is especially true while they don't branch from each other but instead have the same commits. *Right now, master is ahead of 1.0 and 2.0 by 30 commits. Run a git rebase master against them! ;-)

Thoughts?

[robfrawley]

Having spoke with and asked @lsmith77 to enable the wiki, a formal specification for a 2.x release will now be referenced from the wiki and developed using the project feature of the repo.

You can find an initial wiki landing page at liip/LiipImagineBundle/wiki and the majority of the task items from the first message of this issue have been migrated to a project development sprint.

As such, I'm closing this my original issue here.

Please reach out if you would like to be involved in helping create the specification for the next major release or would like to offer development time in achieving our goals.