-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
Allow integer log levels #3
Merged
Merged
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
8233f95
Allow integer log levels
jonathanjfshaw 3b01f4d
Fix coding standards
jonathanjfshaw 6cad040
Test on php 8 only
jonathanjfshaw 8dcba68
Require php 8
jonathanjfshaw 27d1c31
levelMap private
jonathanjfshaw 7ee4d26
Fix review points in TestLogger
jonathanjfshaw 436295a
Add a test
jonathanjfshaw f04316c
Update php version in readme
jonathanjfshaw 7edf059
Whitespace in test
jonathanjfshaw 18ca37c
Split test
jonathanjfshaw 3f3cdf8
Fix coding standards
jonathanjfshaw 6a941ab
More coding standards
jonathanjfshaw 650d13d
Fix coding standards in test
jonathanjfshaw 33ea077
Allow level maps to define keys in any order
colinodell 17f02d9
Remove levelMap functionality
colinodell 79ce374
Throw an exception on unsupported log levels
colinodell 45ab7eb
Minor code tweaks
colinodell 4faa982
Update CHANGELOG
colinodell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to
semver
this means it should be version2.0.0
? 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
semver doesn't tie you to a particular PHP version support cycle, it's up to the project to determine what counts as a breaking API change and document it, and if PHP version increases don't count, then they don't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, a bump in dependencies is not a BC break - see https://github.com/semver/semver/blob/d460299638d3d2af6fcf809285dc6a469e15742a/semver.md#what-should-i-do-if-i-update-my-own-dependencies-without-changing-the-public-api
The only reason we might need to release this as a new major version would be if we BC in the public API. Although the API signature of
TestLogger
did change, all of those changes are backward-compatible:final
, so we don't need to worry about child classes suddenly becoming incompatiblestring|int
types will continue to acceptstring
inputs with no changes needed by end users.This shouldn't be an issue as Composer will prevent the installation of newer versions with incompatible dependencies.
For example, this change will be released as v1.2.0, so I imagine Drupal will use a constraint like
^1.2
. If I were to release v1.3.0 in the future with a new minimum PHP version of8.2.0
, Composer would still allow v1.2.0 on PHP 8.0 and 8.1 - only users on 8.2 could install v1.3.0. And because it would be a minor version bump and we follow semver, there would be no breaking changes to the API. This means that both versions would function identically as far as Drupal is concerned.Bumping dependencies (including minimum PHP versions) in a minor release is done by many other large PHP projects, including Doctrine: https://www.doctrine-project.org/2017/07/25/php-7.1-requirement-and-composer.html
TL;DR: Everything will "just work" :)