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

Fix #1559 - Backported from #1561 #1572

Merged
merged 2 commits into from
Jan 11, 2019
Merged

Fix #1559 - Backported from #1561 #1572

merged 2 commits into from
Jan 11, 2019

Conversation

jmsche
Copy link

@jmsche jmsche commented Dec 18, 2018

Fixes #1559, replaces #1561

@jmsche jmsche mentioned this pull request Dec 18, 2018
@ruflin
Copy link
Owner

ruflin commented Dec 18, 2018

Thanks @jmsche It would be great if @bdlabs could confirm this change to make sure what we merge works for his issue.

@jmsche
Copy link
Author

jmsche commented Dec 20, 2018

Sorry I missed your request for an updated CHANGELOG (flu & angina do not help).

That's fixed!

@romaricdrigon
Copy link

I confirm the fix works on PHP 5.6, could it be merged please @ruflin ?

@p365labs p365labs merged commit 2926265 into ruflin:2.x-temp Jan 11, 2019
@jmsche
Copy link
Author

jmsche commented Jan 14, 2019

Could you please create a release for this patch? Thanks.

@ruflin
Copy link
Owner

ruflin commented Jan 14, 2019

I just created tag 2.3.3.

@jmsche
Copy link
Author

jmsche commented Jan 14, 2019

Thank you :)

@ruflin
Copy link
Owner

ruflin commented Jan 14, 2019

Can you please test it and report back if things work as expected? @romaricdrigon Would be great to get your feedback too.

@jmsche
Copy link
Author

jmsche commented Jan 14, 2019

@ruflin Seems the release was not published to packagist somehow, could you force it? Thanks :)

@ruflin
Copy link
Owner

ruflin commented Jan 14, 2019

What you mean by packaging? composer?

@jmsche
Copy link
Author

jmsche commented Jan 14, 2019

Packagist or composer yes, see https://packagist.org/packages/ruflin/elastica where the 2.3.3 version has not been published here

@romaricdrigon
Copy link

The tag is no published yet on Packagist (https://packagist.org/packages/ruflin/elastica), I couldn't update to test it yet. I believe you should have a "force update" in your Packagist backend, if you are registered as maintainer over there.

@ruflin
Copy link
Owner

ruflin commented Jan 14, 2019

I logged in and hit update. Let's see if that helped.

@romaricdrigon
Copy link

Great, I could update to 2.3.3. Everything is working correctly on PHP5.6.

@Tobion
Copy link
Collaborator

Tobion commented Sep 11, 2019

hey, what's the reason this was merged into branch https://github.com/ruflin/Elastica/tree/2.x-temp but not https://github.com/ruflin/Elastica/tree/2.x ? Can we merge the branches and delete the "temp" one?

@Tobion
Copy link
Collaborator

Tobion commented Sep 11, 2019

Or just delete both 2.x branches because they are not maintained anymore?

@ruflin
Copy link
Owner

ruflin commented Sep 12, 2019

If I remember correctly, one changes the error behviour, the other one doesn't and I didn't feel comfortable realeasing it. Unfortunately it made it into 2.x and I did not want to break users which rely on this branch but still do a release with an other bugfix, that is how 2.x-temp came into life as the "release branch".

Even though both are not maintained, I guess there are still users out there relying on it. I'm +1 on merging this in 2.x if it helps.

@Tobion
Copy link
Collaborator

Tobion commented Sep 12, 2019

I cleaned up some merged and outdated branches yesterday. In general I think it helps maintainance to delete branches when they are not relevant anymore.
I could not figure out what 2.x-temp is meant to be. And the latest 2.x tag https://github.com/ruflin/Elastica/releases/tag/2.3.2 does not match any of the 2.x branches. So I'm a little confused. But I guess it does not matter much anymore for an old branch like this.

@ruflin
Copy link
Owner

ruflin commented Sep 16, 2019

I think both are still needed:

  • 2.x-temp: To do future 2.x releases. Even though they are EOL, normally if a user goes through the hassle to fix a but there, I still do a release or at least we get into the branch
  • 2.x: Contains a change I do not want to release in 2.x but I know some users depend on it.

So the naming is super confusing and 1 commit should have never been merged into 2.x and 2.x should be what 2.x-temp is today 🤦‍♂

Hopefully 2.3.2 is a bit behind 2.x-temp 🤞

Did you clean up some Elastica branches? Good to know :-D

olivier34000 pushed a commit to talkspiritlab/Elastica that referenced this pull request Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants