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

Resolve issues with Symfony dependencies #22

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

Conversation

djmattyg007
Copy link

This commit removes the dependency on the Symfony Config component. The
only code other than getters that was actually being used was the
constructors. In symfony 2, these were incredibly basic. In symfony 3,
they perform some validation.

This validation was causing a couple of "does not exist" tests to fail,
as the Symfony Config module was performing these checks before the
Lurker module, and throwing a different exception. Removing this
dependency neatly solves this issue at the same time.

This commit removes the dependency on the Symfony Config component. The
only code other than getters that was actually being used was the
constructors. In symfony 2, these were incredibly basic. In symfony 3,
they perform some validation.

This validation was causing a couple of "does not exist" tests to fail,
as the Symfony Config module was performing these checks before the
Lurker module, and throwing a different exception. Removing this
dependency neatly solves this issue at the same time.
@djmattyg007
Copy link
Author

Please note that until #24 is merged as well, the tests will continue to fail despite these improvements.

@totten
Copy link

totten commented Oct 4, 2020

Belated cheers @djmattyg007. I combined this with another/similar patch to remove symfony/event-dispatcher -- published as lurkerlite. This should be less picky about dependencies.

@djmattyg007
Copy link
Author

You should merge #24 too, as that should fix the issues you saw with inotify tests. Do you think you'd be able to open the issue tracker on your repo?

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