Skip to content
This repository was archived by the owner on Nov 27, 2020. It is now read-only.

Update SE for Symfony 3.0 #850

Merged
merged 4 commits into from
Sep 23, 2015
Merged

Update SE for Symfony 3.0 #850

merged 4 commits into from
Sep 23, 2015

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Sep 16, 2015

No description provided.

@fabpot fabpot force-pushed the towards-30 branch 2 times, most recently from e6a04cb to 5ce1803 Compare September 16, 2015 08:02
@gnugat
Copy link

gnugat commented Sep 16, 2015

Test classes could be autoloaded only in a development environment by moving them from src/AppBundle/Tests to tests and adding a autoload-dev section in composer.
What do you think about this suggestion?

@stof
Copy link
Member

stof commented Sep 16, 2015

I agree with @gnugat here. Separating tests from the runtime source is a good idea IMO

/app/config/parameters.yml
/app/logs/*
!app/cache/.gitkeep
!app/logs/.gitkeep
/app/phpunit.xml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be changed too as we moved the phpunit config at the root

@stof
Copy link
Member

stof commented Sep 16, 2015

this also requires an update of the HttpKernel component to change the location of the cache and logs

@fabpot fabpot force-pushed the towards-30 branch 2 times, most recently from f05ffa9 to 9dbbaae Compare September 16, 2015 10:31
@weaverryan
Copy link
Member

@gnugat @stof @fabpot is just making the structure match what was decided in #584 - so we should focus only on that (and open a new issue about tests for discussion).

@Tobion
Copy link
Contributor

Tobion commented Sep 17, 2015

Created #855 for tests

@fabpot fabpot changed the title [WIP] Update SE for Symfony 3.0 Update SE for Symfony 3.0 Sep 22, 2015
@fabpot
Copy link
Member Author

fabpot commented Sep 22, 2015

Needs more work

@fabpot fabpot force-pushed the towards-30 branch 4 times, most recently from 4b7d8c1 to 93ec6da Compare September 22, 2015 17:27
@@ -1,11 +1,10 @@
/web/bundles/
/app/bootstrap.php.cache
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is moved to /var/ as well

@Tobion
Copy link
Contributor

Tobion commented Sep 22, 2015

I'd recommend to merge #854 first.

@@ -7,8 +7,8 @@

set_time_limit(0);

require_once __DIR__.'/bootstrap.php.cache';
require_once __DIR__.'/AppKernel.php';
require_once dirname(__DIR__).'/app/bootstrap.php.cache';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As others have said, this lives in /var - and the same change will need to go in the front controllers.

@fabpot fabpot force-pushed the towards-30 branch 2 times, most recently from f50ffc5 to 5262286 Compare September 23, 2015 07:58
@fabpot
Copy link
Member Author

fabpot commented Sep 23, 2015

Besides the new directory structure, I've made another change that should help: sessions are now stored outside of the cache/ directory, and directly into the var/ dir.

@Pierstoval
Copy link
Contributor

@fabpot Why is it better to store the sessions in var instead of cache ?

@fabpot
Copy link
Member Author

fabpot commented Sep 23, 2015

@Pierstoval Because clearing the session store should be decoupled from clearing the Symfony cache, the two are not related. And when using cache:clear, all the cache is flushed. Also, storing sessions in the cache has some problems with Windows (see symfony/symfony#15862 -- not confirmed though)

@fabpot
Copy link
Member Author

fabpot commented Sep 23, 2015

This one is ready for a last review before merge.

@@ -1,6 +1,7 @@
#!/usr/bin/env php
<?php

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure about the file permissions change? Oups.

@michaelcullum
Copy link

Should var/sessions/ not also have a .gitkeep and be added to the .gitignore?

@fabpot
Copy link
Member Author

fabpot commented Sep 23, 2015

@michaelcullum right, fixed

@fabpot fabpot merged commit dcf4ead into symfony:master Sep 23, 2015
fabpot added a commit that referenced this pull request Sep 23, 2015
This PR was merged into the 3.0-dev branch.

Discussion
----------

Update SE for Symfony 3.0

Commits
-------

dcf4ead move sessions outside of the cache directory
693e1f7 moved to the 3.0 directory structure
4a86c13 removed obsolete files
43e379e updated deps to make it possible to install SE with Symfony 3.0
Tobion added a commit that referenced this pull request Oct 28, 2015
This PR was merged into the 2.8 branch.

Discussion
----------

Use a proper version constraint

Any reason why this wasn't used already? I noticed this when looking at #850.

Commits
-------

8d04960 Use a proper version constraint
Tobion added a commit that referenced this pull request Nov 29, 2015
This PR was merged into the 3.0-dev branch.

Discussion
----------

Use Kernel::getEnvironment() to keep consistency

Hi there!

The PR #850 introduced the use of the variable `$this->environment` to get the environment, but we already use the method `Kernel::getEnvironment()`, this PR fixes this to keep consistency.

Please review this before the Symfony 3.0 release.

Commits
-------

d7320f7 Use Kernel::getEnvironment() to keep consistency
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.