Skip to content

Conversation

@jnoordsij
Copy link
Contributor

Requires #45968 to be merged first.

This MR adds the publicPath function the the Application contract for consistency with other path-related helpers. It also updates the global helper function to use this function.

@jnoordsij
Copy link
Contributor Author

Tests suggest I've hit a weird spot here: path.public can be overridden as singleton in the Applications class, which does (before this MR) change the behavior of the public_path helper but not the publicPath function; this seems somewhat inconsistent. Moreover, there's various other paths that can be set, with some of them ignored in both the helper and Application function (e.g. path.config) and some of them being used in both places (e.g. path.storage).

Looking for a bit of input here, but I think the best take would be to create a publicPath property and setter, similar to those for config, and use those consistently.

@jnoordsij jnoordsij force-pushed the add-public-path-to-application-contract branch from 79b9791 to 167276e Compare February 7, 2023 14:51
@jnoordsij jnoordsij force-pushed the add-public-path-to-application-contract branch from 167276e to c5b0a1b Compare February 7, 2023 15:02
@jnoordsij jnoordsij marked this pull request as ready for review February 7, 2023 15:11
@jnoordsij
Copy link
Contributor Author

I've limited this MR to just adding the method to the contract and moving the function definition to inside the Application class; working on consistency of path overriding in https://github.com/jnoordsij/laravel-framework/tree/rework-application-public-path-helper.

@taylorotwell taylorotwell merged commit 4dd8d82 into laravel:10.x Feb 7, 2023
@jnoordsij jnoordsij deleted the add-public-path-to-application-contract branch February 7, 2023 15:25
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.

2 participants