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

[Bundles] Rename getPublicPath() as getPublicDir() #32452

Merged
merged 1 commit into from Jul 12, 2019

Conversation

Projects
None yet
7 participants
@javiereguiluz
Copy link
Member

commented Jul 9, 2019

Q A
Branch? 4.4
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR I'll add this if approved

While documenting #31975 (see symfony/symfony-docs#11930) I realized that the getPublicPath() method name is not consistent with the rest of Symfony.

In Symfony, "path" is usually associated to routes and we use "dir" for things similar to this:

So, this PR proposes to rename getPublicPath() as getPublicDir()

@javiereguiluz javiereguiluz added this to the next milestone Jul 9, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

One difference though is that getCacheDir() and getLogDir() are returning absolute paths. Here, we return relative paths.

@javiereguiluz

This comment has been minimized.

Copy link
Member Author

commented Jul 9, 2019

I haven't thought about that 🤔 To me, "dir" can mean both relative and absolute, such as in this example:

composer-dir

But if this is confusing to people, we can close this.

@linaori

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

As dir stands for directory, it doesn't assume absolute or relative paths

A directory is a "folder", a place where you can put files or other directories (and special files, devices, symlinks...). It is a container for filesystem objects.

A path is a string that specify how to reach a filesystem object (and this object can be a file, a directory, a special file, ...).

Example: you have (probably, depending on your system) a file where system messages are logged, called syslog.

[ ... ]

https://unix.stackexchange.com/questions/131561/what-is-the-difference-between-path-and-directory

@fabpot

fabpot approved these changes Jul 9, 2019

Copy link
Member

left a comment

Fair enough

@trigger_error('Not defining "getPublicPath()" method is deprecated since Symfony 4.4 and will not be supported in 5.0.', E_USER_DEPRECATED);
$publicPath = 'Resources/public';
if (!method_exists($bundle, 'getPublicDir')) {
@trigger_error(sprintf('Not defining "getPublicDir()" method in the "%s" class is deprecated since Symfony 4.4 and will not be supported in 5.0.', get_class($bundle)), E_USER_DEPRECATED);

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Jul 11, 2019

Member

\get_class, as reported by fabbot

@nicolas-grekas
Copy link
Member

left a comment

GTM once fabbot is green :)

@fabpot

fabpot approved these changes Jul 12, 2019

@fabpot fabpot force-pushed the javiereguiluz:continue_31975 branch from 65ea04b to 4ab2f99 Jul 12, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

Thank you @javiereguiluz.

@fabpot fabpot merged commit 4ab2f99 into symfony:4.4 Jul 12, 2019

0 of 3 checks passed

fabbot.io Some changes should be done to comply with our standards.
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

fabpot added a commit that referenced this pull request Jul 12, 2019

minor #32452 [Bundles] Rename getPublicPath() as getPublicDir() (javi…
…ereguiluz)

This PR was squashed before being merged into the 4.4 branch (closes #32452).

Discussion
----------

[Bundles] Rename getPublicPath() as getPublicDir()

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | -
| License       | MIT
| Doc PR        | I'll add this if approved

While documenting #31975 (see symfony/symfony-docs#11930) I realized that the `getPublicPath()` method name is not consistent with the rest of Symfony.

In Symfony, "path" is usually associated to routes and we use "dir" for things similar to this:

* `getCacheDir()` and `getLogdir()` to override Symfony structure (https://symfony.com/doc/current/configuration/override_dir_structure.html)
* `binDir`, `configDir`, `srcDir`, `varDir`, `publicDir` in Symfony Flex recipes (https://github.com/symfony/recipes) to override the dir structure

So, this PR proposes to rename `getPublicPath()` as `getPublicDir()`

Commits
-------

4ab2f99 [Bundles] Rename getPublicPath() as getPublicDir()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.