Skip to content

Conversation

@SurferJeffAtGoogle
Copy link
Contributor

use Symfony\Component\HttpFoundation\Response;

// Use UTC time.
date_default_timezone_set('UTC');
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than set the system-wide default, we should set the timezone on a per-date basis like this:

$date = new DateTime();
$date->setTimezone(new DateTimeZone('UTC'));
//...
    ['timestampValue' => $date->format('Y-m-d\TH:i:s\Z')]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@bshaffer
Copy link
Contributor

Did we look into using tomwalder/php-gds at all for this sample?

@SurferJeffAtGoogle
Copy link
Contributor Author

I didn't look at using using tomwalder/php-gds. I haven't seen us use third-party code like this in samples before. But that library looks good. Should I try using it in a follow-up PR?

{
"require": {
"silex/silex": "^1.3",
"google/apiclient": "~2.0@dev"
Copy link
Contributor

Choose a reason for hiding this comment

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

@bshaffer Do we still need @dev?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dropped.

@tmatsuo
Copy link
Contributor

tmatsuo commented Jun 10, 2016

LGTM

@SurferJeffAtGoogle SurferJeffAtGoogle merged commit 6ed5946 into master Jun 13, 2016
@SurferJeffAtGoogle SurferJeffAtGoogle deleted the squashDatastore branch June 13, 2016 17:38
@bshaffer
Copy link
Contributor

@SurferJeffAtGoogle I think it's worth looking into for a follow up PR. If we did use php-gds, we'd want to factor our other usages of datastore in pubsub and datastore as well.

This is definitely LGTM!

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.

3 participants