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

clean up Paths::$viewDirectory property #1626

Merged

Conversation

samsonasik
Copy link
Member

up to Views directory can be one up directory and /Views from current Config directory, no need to one more up.

__DIR__ . '/../Views';

is enough.

Checklist:

  • Securely signed commits

@samsonasik samsonasik force-pushed the clean-up-paths-viewdirectory branch 2 times, most recently from 0e17cef to 5126afa Compare December 28, 2018 19:42
@jim-parry
Copy link
Contributor

This feels odd. if
public $appDirectory = DIR . '/../../app';
then would it not make sense that the view directory would remain as
public $viewDirectory = DIR . '/../../app/Views';
??
Your change places the Views folder at the same level as app, which is not correct.

@jim-parry jim-parry changed the title clean up Paths::$viewDirectory property WIP clean up Paths::$viewDirectory property Jan 1, 2019
@samsonasik samsonasik force-pushed the clean-up-paths-viewdirectory branch from 5126afa to 6034779 Compare January 2, 2019 11:14
@samsonasik
Copy link
Member Author

Ok, I've updated Paths::$appDirectory as well as well

@jim-parry
Copy link
Contributor

My apologies - I think you were right all along.
I incorrectly assumed that DIR referred to the script being executed initially, namely index.php.
$appDirectory as modified is better, as it does not assume that the folder is called "app".

I'm going to revisit the user guide and shoot for a clearer explanation :)

@jim-parry jim-parry merged commit 936f807 into codeigniter4:develop Jan 2, 2019
@jim-parry jim-parry changed the title WIP clean up Paths::$viewDirectory property clean up Paths::$viewDirectory property Jan 2, 2019
@samsonasik samsonasik deleted the clean-up-paths-viewdirectory branch January 3, 2019 10:49
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