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

Generate an APC prefix based on __FILE__ #657

Merged
merged 1 commit into from
Jul 19, 2014
Merged

Generate an APC prefix based on __FILE__ #657

merged 1 commit into from
Jul 19, 2014

Conversation

trsteel88
Copy link
Contributor

Closes #654

@toaotc
Copy link

toaotc commented Jun 2, 2014

i don't believe this is a good idea. If you have many symfony2 installations on the server, you will have conflicts because same hash would be created for files with same content.

so 👎

@stof
Copy link
Member

stof commented Jun 2, 2014

it does not create the hash based on the content of the file, but based on the path of the file

@toaotc
Copy link

toaotc commented Jun 2, 2014

@stof yes - you're right. I could swear I saw a file_get_contents(). So it seems to be 👍 to me.

edit: but a line comment for what happens there would be nice

@fabpot
Copy link
Member

fabpot commented Jul 8, 2014

@trsteel88 Can you revert the comment and tweak it to explain what's going on here?

@stof
Copy link
Member

stof commented Jul 8, 2014

I'm 👍 for this change if we add the comment back (tweaking it a bit of course)

@pyrech
Copy link

pyrech commented Jul 8, 2014

I wonder if the hash could not be added in the file during the composer install rather than calculated every time? sha1 is not quiet expensive but could be worth it in a prod environnement.

@trsteel88
Copy link
Contributor Author

I am away on holiday at the moment so would not be able to make the change
for a few weeks.

@fabpot
Copy link
Member

fabpot commented Jul 19, 2014

Thank you @trsteel88.

@fabpot fabpot merged commit 3be1eee into symfony:master Jul 19, 2014
fabpot added a commit that referenced this pull request Jul 19, 2014
This PR was merged into the 2.6-dev branch.

Discussion
----------

Generate an APC prefix based on __FILE__

Closes #654

Commits
-------

3be1eee Generate an APC prefix based on __FILE__
@fabpot
Copy link
Member

fabpot commented Jul 19, 2014

I've re-added the comment in 2df0e6f

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.

5 participants