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

Changed namespace #19

Merged
merged 5 commits into from
Apr 6, 2016
Merged

Changed namespace #19

merged 5 commits into from
Apr 6, 2016

Conversation

jrbasso
Copy link
Member

@jrbasso jrbasso commented Mar 25, 2016

Moving the namespace from Zumba\Util to a namespace just for the project (Zumba\JsonSerializer). I am planning to add more classes, interfaces and traits, which will add things that are not necessarily part of the Util namespace.

Also, other open source projects that we have also have dedicated namespaces.

PS: The JsonSerializer class is the same, just changing the namespace at the top and the exception namespace. It is just how GitHub shows the file change. On the repo it was a file rename.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.778% when pulling 1a0f126 on new-namespace into 6bde2e7 on master.

@@ -27,7 +27,7 @@
"autoload": {
"psr-4": {
"Zumba\\": "src/",
Copy link
Member

Choose a reason for hiding this comment

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

@jrbasso If you specify JsonSerializer here, you can move the actual class to the root of the src/ directory (per PSR4).

Copy link
Member Author

Choose a reason for hiding this comment

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

@cjsaylor As we talked in person, this was left intentionally to keep the BC with the existent namespace. I marked as deprecated on this version.

@cjsaylor
Copy link
Member

@jrbasso Also, needs a rebase.

@cjsaylor
Copy link
Member

What do you think in removing the BC compatibility and just adding a note about it in either the readme or adding a changelog file? If we bump the major version it should be known that there are BC breaking changes.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.849% when pulling f33ec4b on new-namespace into 4a67f9b on master.

@jrbasso
Copy link
Member Author

jrbasso commented Apr 6, 2016

@cjsaylor Added a changelog file and rebased. 👍

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.849% when pulling 53a8470 on new-namespace into 4a67f9b on master.

@cjsaylor cjsaylor merged commit fffa0d4 into master Apr 6, 2016
@cjsaylor cjsaylor deleted the new-namespace branch April 6, 2016 02:14
@jrbasso jrbasso mentioned this pull request Jun 11, 2016
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.

None yet

3 participants