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

Merge branch '1.0' into 4.6 #85

Closed
wants to merge 2 commits into from
Closed

Merge branch '1.0' into 4.6 #85

wants to merge 2 commits into from

Conversation

glye
Copy link
Contributor

@glye glye commented Nov 28, 2024

Copy link
Contributor

@ViniTou ViniTou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but please avoid branch names like 1.0 for doing merges.

@glye
Copy link
Contributor Author

glye commented Nov 28, 2024

@ViniTou What's preferred? "Merge remote-tracking branch 'origin/1.0' into 4.6"?

@ViniTou
Copy link
Contributor

ViniTou commented Nov 28, 2024

@glye
Scratch it, I didnt realise that we have 1.0 as stable branch in post-install.

Edit: But i still have my doubts, as I believe there is a chance that 1.0 branch will be deleted after this merge.

Copy link
Contributor

@mnocon mnocon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think something is wrong here.

Ibexa DXP 3.3 uses post-install 1.0:
https://github.com/ibexa/oss/blob/3.3/composer.json#L30

And the 1.0 branch should keep being a branch dedicated for 3.3

Right now it contains merged changes from 4.6:
https://github.com/ibexa/post-install/tree/1.0

So when we tag the package for 3.3 (from 1.0 branch) I think it will be incorrect!
For example, this should say https://github.com/ibexa/post-install/blob/1.0/composer.json#L63 1.0.x-dev, the PHP version is bumped to 7.4 as well

@glye
Copy link
Contributor Author

glye commented Nov 28, 2024

Not sure what happened here. This PR looks OK to me, but the 1.0 branch looks borked indeed.

@ViniTou
Copy link
Contributor

ViniTou commented Nov 28, 2024

@glye
Didnt you maybe - merge 4.6 to 1.0, then during conflict resolution modified everything to remove changes from 4.6 and now try do merge it back again to 4.6? Anyway, this needs to be redone.

@ViniTou ViniTou self-requested a review November 28, 2024 11:07
@alongosz
Copy link
Member

@glye yes, 1.0 is essentially broken now. Moreover if you create PR merging 4.6 from 1.0, there's a chance that 1.0 gets automatically deleted after merging this PR. If you need to make a merge up PR, never use stable branch as a source, create tmp version of it.

@glye
Copy link
Contributor Author

glye commented Nov 28, 2024

That's what I get from being stressed and following github instructions to the letter. Discussing fixes with Dawid & Marek

@glye
Copy link
Contributor Author

glye commented Nov 28, 2024

New, separate PR for 4.6: #86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants