-
Notifications
You must be signed in to change notification settings - Fork 8
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
Move source classes to the Yoast\WHIPv2
namespace
#157
Conversation
1089606
to
7d14d01
Compare
Pull Request Test Coverage Report for Build 7300692411
💛 - Coveralls |
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.
@enricobattocchi I've looked this over.
As mentioned on Slack, the directory names need to be updated to match the namespace in a case-sensitive manner.
Looks like you made this change already, but only for the directories containing namespaced classes, which makes the directory structure inconsistent, so I propose to also apply this for the directories containing plain files.
Another thing is that, yes, you've added namespaces, but the prefixing which was applied to the classes (for lack of namespacing) was not removed.
The class prefixing is no longer needed now and as this change is going into a major release, now would be the time to remove it.
I'll push some commits in a minute to fix this all up and apply some minor changes to the PHPCS config as well.
Let me know what you think.
Also includes backslashing of the global namespace functions
a57657b
to
8e96f58
Compare
Looks good to me! |
@enricobattocchi To keep the history cleaner, shall I squash those rename commits ? (or will you ?) I mostly did them one class at the time to make sure I didn't miss anything, but these don't really need separate commits. |
On it! |
8e96f58
to
b52f464
Compare
Also includes backslashing of the global namespace functions.