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

require beberlei/doctrineextensions for production #58

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

phpwutz
Copy link

@phpwutz phpwutz commented Feb 28, 2018

by default, installing dtc_queue will not install the doctrine extensions which in turn causes the Trends-Screen to not load any data (due to missing DAY, MONTH etc. functions)

by default, installing dtc_queue will not install the doctrine extensions which in turn causes the Trends-Screen to not load any data (due to missing DAY, MONTH etc. functions)
@mmucklo
Copy link
Owner

mmucklo commented Mar 4, 2018

The doctrine extensions only matter for people using the ORM mode. Would perhaps a message on that screen instructing people to install the extension be more appropriate?

The reason being is I don't necessarily want to force people not using an ORM-based queue manager to install the extensions...

Your thoughts?

@phpwutz
Copy link
Author

phpwutz commented Mar 4, 2018

I agree on the concern to install dependencies that might not be needed by the user. My concern was rather to prevent code that works in development environment breaking in production.

Personally, I think a library that promises to work with doctrine should either
a) not use vendor-specific database functions or
b) at least provide the corresponding ORM-extension by default

As a) is not really an option, I would think the lower entry-hurdle of being able to "just install" this package is worth the overhead of having beberlei/doctrineextensions installed on some systems that might not need it. In case I am the ORM-using exception here, I am also willing to change my opinion on the "good default".

Here is a compromise suggestion:
What do you think about moving beberlei/doctrineextensions to the "suggest" block in composer.json?
This way it also won't work by default in dev environment and show the user a message with the recommendation on installing this package.

@mmucklo
Copy link
Owner

mmucklo commented Mar 5, 2018

It's in the suggest section:

        "beberlei/DoctrineExtensions": "Alternative for YEAR, MONTH, DAY, HOUR, MINUTE if using JobTiming trends",

Seems to be Camelcased up on packagist
(https://packagist.org/packages/beberlei/DoctrineExtensions)

The alternative might be to either roll my own Type extensions or figure out some other trick to do what I need... I'm open to other SQL query suggestions...

@mmucklo
Copy link
Owner

mmucklo commented Mar 6, 2018

@phpwutz - what do you think given my comment above?

@phpwutz
Copy link
Author

phpwutz commented Mar 6, 2018

The alternative might be to either roll my own Type extensions

i fail to see how shipping a custom version of type extensions would be beneficial over shipping a known-to-be-working library that provides the same extensions => although I'd also be happy to see that change for the reason below.

My only intention here was to give people a better first time experience so that after following the installation instructions one would have all advertised features working. Maybe a simple README-update would also do the trick

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.

2 participants