User tests: Successful: Unsuccessful:
This PR fixes some more class/method/function names to comply with the CamelCase rules mentioned here:
http://joomla.github.io/coding-standards/?coding-standards/chapters/php.md
...be written in CamelCase even if using traditionally uppercase acronyms (such as XML, HTML).
Also see: joomla/joomla-framework#377
Status | New | ⇒ | Pending |
Labels |
Added:
?
?
|
If you're referring to the unit tests don't worry about renaming files there right now.
Ah I see what you did. Travis catched it
Keep in mind that on Linux systems, filenames are case sensitive and thus they have to match exactly.
On Windows systems, that is another story, there it wouldn't matter.
Renaming the files is out of topic, since on Linux we would end up having two files, while on Windows we only have one.
Ok, so should I not commit the filename fixes in the test folder?
@Bakual The unit test classes don't get packaged, so the issue we hit with the loader will never occur for 99.9% of people. Like I mentioned above, there's really no need to rename the test files. If you're adamant about doing it though, what I usually do is something like this:
git mv path/to/OldFile.php Oldfile.php && git mv Oldfile.php path/to/Oldfile.php
Yeah, I found out how to change the filenames :)
This PR isn't about fixing autoloading so much. More about fixing code styling, So good to have the correct casing throughout, also in test suite.
So good to have the correct casing throughout, also in test suite.
Not gonna argue that at all. But because of the conventions used in some places in the CMS, there may be places where an acceptable class/method name doesn't align with the code style preferences (hence my comment on the SEOHtml
thing), so anything testing on those behaviors should remain in place. Otherwise, as long as we aren't breaking things, I see no issue with these PRs.
Ok, great :)
So hence a little testing is required on this PR. Not sure where/how to test the ExampleViewSeoHtml class though...
The unit test classes don't get packaged, so the issue we hit with the loader will never occur for 99.9% of people.
True, files in the test folder should be fine to be renamed.
Just don't do it on files that get packed for production
Ok, all done and good now :)
Category | ⇒ | Code style |
Why do we have here a Tag "Language Change" ? I see no ini modified or added.
I think because it touches files like administrator/language/en-GB/en-GB.localise.php
It's an automatic tag based on the folder
Ya, it was a purposeful thing to make the code that adds that label scan for all changes in the language folders versus just the INI files. For example, if something significant changes in the localise.php files or the XML files, for those who are following language changes in the repo and not waiting for the language freeze to be given a diff, they can also check those extra files to see if some change applies to them.
OK, got it. Did not see the change in localise.php
-abstract class En_GBLocalise
+abstract class En_GbLocalise
Will it be necessary to change all our xx-XX.localise.php or is this B/C?
It's B/C. Since these classes aren't autoloaded, you could write it as En_gBlocALIse
and PHP could care less. It's just a matter of code standards and consistency.
If they were autoloaded either using JLoader
or Composer's autoloader, there'd be a little more strictness on class and file names matching a convention.
Thanks for the explanation.
So is this good to go?
Someone has to review it. I haven't had the time yet. If someone can do it, pleae do
Been too long. Out of sync. So closing this.
Effort and time lost.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-01-26 23:58:29 |
Closed_By | ⇒ | nonumber |
What do you mean? Changing filename casing would be a bad idea. We just had an issue with that.