-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Is modification time the best default to calculate the compilation digest? #1611
Comments
relates to #1439 |
@oriolbcn Thanks, that makes sense.
if Rails.env.development
# use mtime
else
# file digest
end // @jpickwell Thanks for reminder and @oriolbcn sorry for late response. |
Yep sounds good I like the approach it would be clearer than now. |
After 95c00e2 this can proably be closed. |
#1522 introduced a neat change, to be able to calculate the compilation digest based on the contents of the file instead of the modification time.
Now I wonder: Wouldn't that calculation (based on contents) be a better default than the modification time? In the end, the contents of the file is what matters and what will always be 100% accurate. The modification time may or may not reflect a change in the file.
Specifically, this doesn't work well with Capistrano, which is probably one of the most (the most?) popular deployment tool for Rails apps. With Capistrano, every time you make a new deploy, it copies over all the app files to a new folder, which makes the files have a different modification time, which defeats the purpose of the digest. Copying files is one of the scenarios when the modification time does not work as an indication of file modification, there may be others.
I understand that currently it is done this way for performance reasons, but is it worth compromising the usefulness of the digest due to some (not a lot, I guess) speed? What I propose is to make the calculation based on the contents by default and maybe ditch the other one completely or make it configurable with a different environment variable or option (like "FASTER_DIGESTS" or something).
The text was updated successfully, but these errors were encountered: