Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade 2.0 from 1.0 #1011

Closed
wants to merge 49 commits into from
Closed

Upgrade 2.0 from 1.0 #1011

wants to merge 49 commits into from

Conversation

dbu
Copy link
Member

@dbu dbu commented Nov 18, 2017

Q A
Branch? 2.0
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? see travis
Fixed tickets -
License MIT
Doc PR -

it seems that 2.0 has been way behind 1.0. i solved a lot of conflicts, there might be issues with some of the resolutions. we also have a couple of deprecated things now that should be removed in the 2.0 branch. but i think that should be done in a separate pull request.

robfrawley and others added 30 commits May 21, 2017 16:31
Move to trusty for travis except for php 5.3
return filter function String
…m-allowed-failures

Remove Symfony 3.3 from allowed failures
…d-of-version-numbers

Use simplified Symfony version comparison operation and CS fixes
[Composer] Allow imagine-library version 0.7.0
- Allow construction of Locator without data roots for BC
- Mark FilesystemLocator::setOptions deprecated
Modify some PHP Annotations
Added support for centerright and centerleft position
@lsmith77
Copy link
Contributor

there are also now a few more commits that need to be merged ..

@lsmith77
Copy link
Contributor

@robfrawley can you take this one on?

@robfrawley
Copy link
Collaborator

I'll try to rebase this properly over the current 2.x commit and then take a look and see if everything looks okay. Likely by the end of the week...

@robfrawley robfrawley added Attn: Blocker This item blocks other issue(s) or PR(s) and therefore must be resolved prior. State: Reviewing This item is being reviewed to determine if it should be accepted. State: Confirmed This item has been confirmed by maintainers as legitimate. and removed State: Reviewing This item is being reviewed to determine if it should be accepted. labels Jan 21, 2018
@pyrech
Copy link
Contributor

pyrech commented Mar 8, 2018

Sorry to bother you but is there an ETA for 2.0?
Because at the moment, there is still no release compatible with Symfony 4.0 😕

@robfrawley
Copy link
Collaborator

robfrawley commented Mar 8, 2018

@pyrech You can reference our milestones page for 2.0 release information, as well as the open issues posing the same question: #1052, #1064. This project has few active maintainers; those of us contributing provide as much of our free time to this project as we can. The 2.0 release is coming shortly. Sorry for the delay.

@pyrech
Copy link
Contributor

pyrech commented Mar 8, 2018

Sorry that I did not look at milestones page, nor the opened issues 😞. Anyway thanks for your answer, I didn't mean to put any pressure on the maintainers. I will try to see if I can help on some issues when I come back from holidays.

@robfrawley
Copy link
Collaborator

@pyrech It's all good. We'll have the release out the door either this coming Monday or the following one.

@maximgubar
Copy link
Contributor

@dbu, @lsmith77, @robfrawley what is blocking this PR to be merged (except failed tests and conflicts) ?

@tifabien
Copy link
Contributor

It seems you released the 2.0.0 without having this PR merged. Is it normal ? This makes the upgrade almost impossible.
Is there anything we can do?

@dbu
Copy link
Member Author

dbu commented May 11, 2018

i don't think this should be rebased - that will lose the git commit ids, and then you can't ever merge the 1.0 branch into 2.0 again when there is a bugfix to the legacy branch.

and yes, this should be merged asap and a 2.0.1 or maybe 2.1.0 be released.

@lsmith77
Copy link
Contributor

@maximgubar can you have a look?

@maximgubar
Copy link
Contributor

@robfrawley, @makasim, @cedricziel, @imanalopher, @antoligy, @rpkamp, @lstrojny, @cmodijk, @dbu

Since this PR was not merged in time and due to its current complexity we propose to split it in small PRs. So basically each author takes his changes and creates a new PR against 2.0 branch.

@robfrawley
Copy link
Collaborator

robfrawley commented May 14, 2018

@maximgubar: That sounds like a solid plan. Once we get the major items merged separately, we can tag a 2.1.0 release and improve backward compatibility.

There are commits and lines of code in this PR that were intended only for the 1.x branch and not 2.x (for example: deprecation notices, the command changes I had made aren't for 2.x, and I even noticed duplicate commits here for features that were submitted independently to the 2.x branch already).

I was intending to take a stab at this as a whole since a plurality of PRs were authored by me, but I don't believe anyone is familiar enough with everyone else's commits to reliably merge this together without guidance. Relying on the original authors to submit PRs to 2.x branch is a reasonable way to manage this process and will help ensure code we don't want in 2.x doesn't accidentally end up there.

I'll get my changes together this week.

@tifabien: Regarding the release of 2.0.0 without this PR, we hit the deadline for tagging a release that was compatible with Symfony 4.x (in fact, we hit the target release date multiple times without a release) and it was important there was a stable release of this bundle with Symfony 4.x support, regardless of complete compatibility. Remember, this project only has two or three individuals that maintain it when they can. Hopefully the recent addition of @maximgubar will help on this front! :-)

While our intention is to create the smoothest upgrade path possible, a new major release like this can of course contain BC breaks. All I would note is that if we are missing an explicit notice about something in our changelog or upgrade files, please advise, as those should accurately reflect differences between 1.x and 2.x. Otherwise, 2.1.0 will be out in the future with additional support for 1.x migrations; hold off until then if required.

@cmodijk
Copy link
Contributor

cmodijk commented May 14, 2018

My pull request is in the 1.0 branch do you want me to create a new pull for the 2.0 branch?

@robfrawley
Copy link
Collaborator

robfrawley commented May 15, 2018

@cmodijk Give it a few days (unless you really want to create the PR yourself in the immediate); many of the changes are extremely minor (such as your PR to add "centerright" and "centerleft" positioning to the background filter loader) and I think I can automate such cases without each individual author's manual handling, which I suspect will end up being diffcult to coordinate between all the various contributors.

@maximgubar This week I happen to have tons of free time and an employer sympathetic to open source software, so I'll handle the vast majority of the updates to 2.0 from 1.0 (using separate PRs as appropriate), and then next weekend I'll tag all users that don't have a straightforward PR from 1.0 and request that they manually re-submit the changes against 2.0. We can give them an additional week, and then plan to tag 2.1.0 on Monday, June 4th (two weeks from today). How does that sound?

@tifabien
Copy link
Contributor

@robfrawley my comment was bad. I meant that when I saw the 2.0 release I tried to upgrade but was surprise to see that some 1.0 feature was not there or removed (that's why I added a missing note in the upgrade guide ;)). I'm thankful for all the great job your are doing for maintaining this bundle!
I think I'm too accustomed with the smooth SF upgrade process :)

@robfrawley
Copy link
Collaborator

I've created a project to manage this process here: https://github.com/liip/LiipImagineBundle/projects/2. I will be updating that with all the required entries later today, and progress can be tracked there. @maximgubar Got the first one for you to review #1096 (glad to have someone who can regularly review things now; there was a period of time where I was reviewing my own contributions, a practice I do not particularly like). :-)

@maximgubar
Copy link
Contributor

since default branch was changed to master, please update the base branch for this PR

@franmomu franmomu mentioned this pull request Aug 3, 2019
@dbu
Copy link
Member Author

dbu commented Aug 9, 2019

done in #1199

@dbu dbu closed this Aug 9, 2019
2.1.0 Merges from Latest 1.x.x automation moved this from To Do to Done Aug 9, 2019
@dbu dbu deleted the upgrade-2.0-from-1.0 branch August 9, 2019 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Attn: Blocker This item blocks other issue(s) or PR(s) and therefore must be resolved prior. State: Confirmed This item has been confirmed by maintainers as legitimate.
Development

Successfully merging this pull request may close these issues.

None yet