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

Remove dev dependency on laminas/laminas-cache-storage-deprecated-factory #142

Conversation

ramchale
Copy link
Contributor

Q A
Documentation /no
Bugfix no
BC Break no
New Feature no
RFC yes
QA no

Description

laminas/laminas-cache-storage-deprecated-factory states "This factory will not provide support for laminas-cache v4+ and is only meant to be used as a temporary solution in combination with other laminas components."

Continued use is likely to make future maintenance of this package difficult

While it is used in one code path in Translator, as it is not a non-dev dependency I think it's safe to assume anyone using this functionality has it listed as one of their dependencies

@ramchale ramchale force-pushed the remove-laminas-cache-storage-deprecated-factory branch from a2d4085 to 65480f1 Compare October 22, 2024 07:39
@ramchale ramchale force-pushed the remove-laminas-cache-storage-deprecated-factory branch from 65480f1 to 93a7418 Compare October 22, 2024 07:43
Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

Overall, not sure about this change.
Effectively, we've just dropped a dev dependency and now have an untested code path that imports symbols not defined as hard dependencies.

The undefined deps are of course not your fault 😅, however, I'm preferring the runtime deprecation of the cache option as mentioned in #134 with the addition of new tests to cover the deprecation notice.

@ramchale
Copy link
Contributor Author

No worries. Thanks for taking a look.

I'll see how the issue conversation goes, and potentially submit a PR based on the outcome of that.

@ramchale ramchale closed this Oct 22, 2024
@froschdesign
Copy link
Member

"…and is only meant to be used as a temporary solution in combination with other laminas components."

laminas-cache-storage-deprecated-factory was created precisely for this component, among others.

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.

3 participants