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

Dependency update #118

Draft
wants to merge 5 commits into
base: 3.x
Choose a base branch
from
Draft

Conversation

hafkenscheid
Copy link

Updated dependencies, fixed dependency related things, added an exception and added phpdocs.

@hafkenscheid
Copy link
Author

For issue #114

@rvanlaak
Copy link
Owner

Thank you for the draft PR. I see it does a lot of different things, from providing phpdoc, to adding infra dependencies to composer.json, and dropping earlier versions of dependencies and their quirks. Imho we should split this to different PRs, to get simple changes merged quickly.

About the phpdoc changes; all the simple typehinted object types will be deducted from the PHP typehints by most IDEs. So, the phpdocs that were added there are not needed when no additional description gets provided. Would you be ok with reverting those changes?

@hafkenscheid
Copy link
Author

I reverted the PHPdoc changes.

Do you also want to split the dependencies and related updates?
In my opinion it is not very useful to check full symfony 3.4 compatibility because it is not being maintained anymore.

@rvanlaak
Copy link
Owner

v3.0.0-beta3 does some household cleanup as well, and introduces Github Actions for testing as Travis stopped running tests. https://github.com/dmishh/SettingsBundle/releases/tag/v3.0.0-beta3

Regarding 3.4 compatibility, does the 3.4 support block you from introducing new features? As long as it takes no effort to keep support, I'm in favor of keeping 3.4 under test coverage.

Could you please revise this PR so we can get it merged?

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