search

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
721 stars 822 forks source link

Naming Inconsistency #10596

Open strarsis opened 1 year ago

strarsis commented 1 year ago

Affected Version

Silverstripe v4.x.

Description

According to 4.x changelog/migration notes the Requirements::combine_files() method was renamed to Requirements::combineFiles(). However, the v4.x SilverStripe guide still uses the method name Requirements::combine_files() instead of the new Requirements::combineFiles(). This is also the case for the v4.x API reference, where some parts still use this old combine_files() method name: https://api.silverstripe.org/4/SilverStripe/View/Requirements_Backend.html#:~:text=Example%20for%20combined%20JavaScript https://api.silverstripe.org/4/SilverStripe/View/Requirements_Backend.html#:~:text=Example%20for%20combined%20CSS

The source search finds occurrences of combine_files (from which also the docs are generated): https://github.com/silverstripe/silverstripe-framework/search?q=combine_files

kinglozzer commented 1 year ago

I think the confusion here is between two different classes which name the methods differently - Requirements::combine_files() and Requirements_Backend::combineFiles(). One just calls the other:

https://github.com/silverstripe/silverstripe-framework/blob/dcdc25500be729f4340d42f8a4fc797461f862f3/src/View/Requirements.php#L392-L398

I think all the docs you’ve linked are actually correct - the 4.x changelog says that Requirements_Backend has the renamed methods (not Requirements), the guides/API docs using Requirements::combine_files() are correct too.

I suppose we could look at making this consistent for 5.x?

strarsis commented 1 year ago

@kinglozzer: Yes, this causes confusion! Making the naming conventions and those names the same among different classes surely would help!