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

LastModified should be in UTC #600

Merged
merged 1 commit into from Nov 19, 2015
Merged

LastModified should be in UTC #600

merged 1 commit into from Nov 19, 2015

Conversation

Bladrak
Copy link
Contributor

@Bladrak Bladrak commented Nov 12, 2015

LastModified date should not depend on local timezones, this PR should fix it. However, this creates a BC Break with other result storages.

@masom
Copy link
Contributor

masom commented Nov 12, 2015

Great find!

Should there be a test that ensures the TZ info is returned to the browser?

@Bladrak
Copy link
Contributor Author

Bladrak commented Nov 12, 2015

Maybe yes, I still need to figure out how to run thumbor's tests on my computer :) I'll try to add some.

@Bladrak Bladrak force-pushed the lm_utc branch 10 times, most recently from 1a8af42 to 80e784c Compare November 18, 2015 15:00
@masom
Copy link
Contributor

masom commented Nov 18, 2015

Poor @Bladrak :D ( we see all the builds on a slack channel )

@Bladrak
Copy link
Contributor Author

Bladrak commented Nov 18, 2015

Haha sorry to spam you, didn't manage to run the tests locally :(

@masom
Copy link
Contributor

masom commented Nov 18, 2015

It's all good.

Yeah the local testing environment is a little bit hard to setup right now. We are removing a bunch of plugins from thumbor to make that process easier. No more mongodb / redis / memcache will greatly help with that.

@Bladrak
Copy link
Contributor Author

Bladrak commented Nov 18, 2015

Yup. I also noticed an issue with NFS sharing for the pil engine (given NFS is case insensitive, it confounds the import between the engine and the module). Maybe there's a rename to perform on the engine?

@masom
Copy link
Contributor

masom commented Nov 18, 2015

I see. We might have to normalize engine names like pil_engine, gif_engine.

@Bladrak
Copy link
Contributor Author

Bladrak commented Nov 18, 2015

Yep, might be better :)

@Bladrak Bladrak force-pushed the lm_utc branch 5 times, most recently from 105d387 to def9ea1 Compare November 19, 2015 12:54
@Bladrak
Copy link
Contributor Author

Bladrak commented Nov 19, 2015

@masom \o/

masom added a commit that referenced this pull request Nov 19, 2015
LastModified should be in UTC
@masom masom merged commit 43698d5 into thumbor:master Nov 19, 2015
@Bladrak Bladrak deleted the lm_utc branch November 19, 2015 14:21
@Bladrak
Copy link
Contributor Author

Bladrak commented Nov 19, 2015

@masom @heynemann could we have a release of thumbor so I can reference this fix as a dependency for the next tc_aws version?

@masom
Copy link
Contributor

masom commented Nov 19, 2015

We're looking at thumbor 6...

There is also a 5.x branch we could backport this fix. We'll have to update all the storages to return UTC.

@Bladrak
Copy link
Contributor Author

Bladrak commented Nov 19, 2015

Yup you're right, thumbor 6 might be soon enough :) Do we know around when it will be released?

@heynemann
Copy link
Member

I'm worried about releasing 6.0.0 due to URL encoding issues.

We need to work a little further on this.

On Thu, Nov 19, 2015 at 12:47 PM, Hugo Briand notifications@github.com
wrote:

Yup you're right, thumbor 6 might be soon enough :) Do we know around when
it will be released?


Reply to this email directly or view it on GitHub
#600 (comment).

christianjgreen pushed a commit to fanhero/thumbor that referenced this pull request Aug 29, 2017
LastModified should be in UTC
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.

None yet

3 participants