Skip to content

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

Merged
merged 1 commit into from
Oct 31, 2020
Merged

Renamed namespace #10

merged 1 commit into from
Oct 31, 2020

Conversation

grogy
Copy link
Member

@grogy grogy commented Oct 1, 2020

It resolve #3

@grogy
Copy link
Member Author

grogy commented Oct 1, 2020

It is a candidate for release 1.0 version. I don't want to create 0.x with BC break. I mean the name PHPParallelLint of the namespace is best. It will be same in other packages.

Does it sense to @jrfnl?

Copy link
Collaborator

@jrfnl jrfnl left a 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).
Copy link
Collaborator

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have the right

@@ -1,5 +1,5 @@
<?php
use JakubOnderka\PhpConsoleColor\ConsoleColor;
use PHPParallelLint\PhpConsoleColor\ConsoleColor;
Copy link
Collaborator

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 ?

@grogy
Copy link
Member Author

grogy commented Oct 1, 2020

Thanks for the link, I copy from semver: MAJOR version when you make incompatible API changes

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.

@jrfnl
Copy link
Collaborator

jrfnl commented Oct 1, 2020

What about create 0.4 version without this PR and 1.0 with only this PR?

In that case, I would recommend going straight to 1.0. There is nothing currently in "unreleased" that affects end-users of this repo, so having a 0.4 with just those changes, adds no value.

@grogy grogy force-pushed the changed-namespace branch from 905d415 to 5b7fdeb Compare October 7, 2020 13:53
@grogy
Copy link
Member Author

grogy commented Oct 7, 2020

Thanks for your feedback. So I will create 1.0 after merge this PR.

Namespace to tests is added ;)

Copy link
Collaborator

@jrfnl jrfnl left a 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).
Copy link
Collaborator

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 to PHP_Parallel_Lint.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have right, thanks

@grogy grogy force-pushed the changed-namespace branch from 5b7fdeb to 6978b17 Compare October 24, 2020 05:54
@grogy grogy merged commit 3c85f7c into master Oct 31, 2020
@jrfnl jrfnl added this to the 1.0.0 milestone Dec 10, 2021
@jrfnl jrfnl deleted the changed-namespace branch February 19, 2022 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Rename the namespace
2 participants