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

[BC hotfix] Revert typo3 installer deletion #300

Merged

Conversation

QuingKhaos
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? yes
Fixed tickets -
Related issues/PRs #281
License MIT

What's in this PR?

Reverting #281 which removed the TYPO3 CMS installer and added a deprecation notice to use the one from @helhum

Why?

Because the deletion of a installer is a majow BC break. It doesn't matter if there is an official one or not, you broke many projects now. Ours definitively and maybe other ones too. Feels like #npmGate 😈

And it is no problem to leave the installer here, as the now official typo3 composer installer has a conflict section against composer/installers, thus no unforeseen side effects.

Deprecations

Deprecated the TYPO3 installer in the README

…po3_extension"

This reverts commit 6aac6a0, reversing
changes made to d7fb3fe.
@helhum
Copy link

helhum commented Apr 6, 2016

Back and forth might cause even more confusion.

But if this is reverted again, then the deprecation message should be visible during build time, not only in the readme

@QuingKhaos
Copy link
Contributor Author

It might cause confusion. But the deletion is a major BC break and causes real problems. Never do this in bugfix releases.

Feel free to add a deprecation to the installer after it got reverted ;)

@helhum
Copy link

helhum commented Apr 6, 2016

It might cause confusion. But the deletion is a major BC break and causes real problems. Never do this in bugfix releases.
Feel free to add a deprecation to the installer after it got reverted ;)

It might be natural for you to value your problems higher than other peoples problems.
But if "real problems" meant, changing a composer.json to require a more specific version of a package, the world would be a better place :-P

Anyway, please do what you consider best, I will not waste more time here.

@QuingKhaos
Copy link
Contributor Author

I care about semantic versioning and backwards compatibility. If we profit from it, yayyy. Everyone profits from semver and caring about BC.

changing a composer.json to require a more specific version of a package
This is not what composer and semver is meant to be.

@helhum
Copy link

helhum commented Apr 7, 2016

No need to comment on BC, semver or compatiblility. Everybody knows how it works…

One last comment however on the reasoning why we from the TYPO3 community consider it an improvement if TYPO3 extensions are not handled any more by composer/installers

We like to be a nice citizen in the PHP community and be operational in combination with other PHP projects.

This means we would like TYPO3 projects to be installable with other projects that require composer/installers. This is not possible with the installer code for TYPO3 extensions which is/was present here, because both installers did conflicting operations in certain situation, which left the TYPO3 installation in an unsusable state.

We mitigated this on our side, by adding a conflict declaration to our installer. But this violates our initial goal of being a good citizen.

We are not aware of any other solution to fully fix this, besides removing the installer code here.

Our final goal is, that TYPO3 will not require any special installer code, but it will take a while until we get there.

Hope this clears things up a bit.

I respect any decision taken here. We can also live with a broken installer here and a conflict declaration on our side, although we would not choose this option.

helhum added a commit to TYPO3/CmsComposerInstallers that referenced this pull request Apr 7, 2016
helhum added a commit to TYPO3/CmsComposerInstallers that referenced this pull request Apr 7, 2016

/**
* Extension installer for TYPO3 CMS
*
Copy link
Member

Choose a reason for hiding this comment

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

Add @deprecated tag with link to new installer, please.

@niksamokhvalov
Copy link
Member

@helhum remove installers this is a major change. It is better to leave the old installer, but make notice of his unfitness. For example, you can make PR, that would show notice during the installation of the package.

Give people time to switch to the new installer ;)

@QuingKhaos QuingKhaos force-pushed the hotfix/revert-typo3-installer-deletion branch from ac7e71b to 24629e9 Compare April 13, 2016 15:12
@QuingKhaos
Copy link
Contributor Author

@niksamokhvalov Added the deprecated annotation to the installer class.

@niksamokhvalov niksamokhvalov merged commit fdabec1 into composer:master Apr 13, 2016
@niksamokhvalov
Copy link
Member

v1.0.25

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.

None yet

3 participants