-
Notifications
You must be signed in to change notification settings - Fork 6
Renamed namespace #10
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
Conversation
It is a candidate for release 1.0 version. I don't want to create 0.x with BC break. I mean the name Does it sense to @jrfnl? |
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.
I don't want to create 0.x with BC break.
Semver-wise, BC-breaks in 0.x versions are allowed, though it would need a new minor, i.e. 0.4
.
Composer treats minors below 1.0
as majors anyway, so it wouldn't break anything unless someone doesn't use proper version constraints, like *
, but then 1.0
would break their package just the same.
Having said that, if you feel the package is mature enough for 1.0
, go for it! I'm all for stable release packages 👍
I mean the name PHPParallelLint of the namespace is best. It will be same in other packages.
As that's in line with the GH organization name, that seems a logical choice. Or, as the GH organisation name has dashes, possibly PHP_Parallel_Lint
.
The only thing which looks a bit weird is the PHP acronym being in uppercase in the top-level name, but in mixed case in the next level... should that be made consistent ?
namespace PHPParallelLint\PhpConsoleColor;
// --- ---
CHANGELOG.md
Outdated
@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). | |||
|
|||
### Internal | |||
|
|||
- BC-Break: Changed - renamed namespace [#10](https://github.com/php-parallel-lint/PHP-Console-Color/pull/10) [@grogy](https://github.com/grogy). |
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.
I'd suggest moving the changelog entry up into a ### Changed
header. This is not an internal change, but a change to the Public API, so should be annotated as such.
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.
You have the right
tests/ConsoleColorTest.php
Outdated
@@ -1,5 +1,5 @@ | |||
<?php | |||
use JakubOnderka\PhpConsoleColor\ConsoleColor; | |||
use PHPParallelLint\PhpConsoleColor\ConsoleColor; |
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.
Would it make sense to add a namespace to this test file as well ? I'm thinking PHPParallelLint\PhpConsoleColor\Tests
would be in line with PSR-4 ?
Thanks for the link, I copy from semver: I mean that the namespace is API for programmers with this library. For it, I want upgrade major version. What about create 0.4 version without this PR and 1.0 with only this PR? Namespace inconsistence - oh, I am missing it. Version with underscores will be better. |
In that case, I would recommend going straight to |
905d415
to
5b7fdeb
Compare
Thanks for your feedback. So I will create 1.0 after merge this PR. Namespace to tests is added ;) |
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.
Left one minor remark, other than that, looks good to go.
CHANGELOG.md
Outdated
@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). | |||
|
|||
## [Unreleased] | |||
|
|||
### Changed | |||
|
|||
- BC-Break: Changed - renamed namespace [#10](https://github.com/php-parallel-lint/PHP-Console-Color/pull/10) [@grogy](https://github.com/grogy). |
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.
To make it more obvious what has changed, you may want to mention the "from" and the "to".
The top-level namespace for all classes has changed from
JakubOnderka
toPHP_Parallel_Lint
.
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.
You have right, thanks
5b7fdeb
to
6978b17
Compare
It resolve #3