Skip to content

iCalendar: use WP time constant instead of non-existing constant.#7163

Merged
eliorivero merged 2 commits intomasterfrom
fix/calendar-time-constant-hour
May 15, 2017
Merged

iCalendar: use WP time constant instead of non-existing constant.#7163
eliorivero merged 2 commits intomasterfrom
fix/calendar-time-constant-hour

Conversation

@jeherve
Copy link
Copy Markdown
Member

@jeherve jeherve commented May 10, 2017

Fixes #7162

WordPress defines a few specific time constants:
https://github.com/WordPress/WordPress/blob/4.7.4/wp-includes/default-constants.php#L120-L125

HOUR_IN_MINUTES is not one of them and is only defined on WordPress.com. Let's not use it.

Proposed changelog entry for your changes:

  • Upcoming Events: use correct time constant to define an hour.

Fixes #7162

WordPress defines a few specific time constants:
https://github.com/WordPress/WordPress/blob/4.7.4/wp-includes/default-constants.php#L120-L125

HOUR_IN_MINUTES is not one of them and is only defined on WordPress.com. Let's not use it in Jetpack.
// generate a DateInterval object from the timezone offset
$gmt_offset = get_option( 'gmt_offset' ) * HOUR_IN_MINUTES;
$gmt_offset = get_option( 'gmt_offset' ) * HOUR_IN_SECONDS;
$timezone_offset_interval = date_interval_create_from_date_string( "{$gmt_offset} minutes" );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this cause any problems with line 100 where it is getting the time difference in minutes? Should that be changed to seconds?
The HOUR_IN_MINUTES constant returns 60, and the HOUR_IN_SECONDS constant returns 3600.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's a good point, nice catch! Updated in ccc4fb9

Copy link
Copy Markdown
Contributor

@briancolinger briancolinger left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels May 11, 2017
@eliorivero eliorivero merged commit 47ef05a into master May 15, 2017
@eliorivero eliorivero deleted the fix/calendar-time-constant-hour branch May 15, 2017 11:24
@eliorivero eliorivero removed the [Status] Ready to Merge Go ahead, you can push that green button! label May 15, 2017
jeherve added a commit that referenced this pull request May 23, 2017
jeherve added a commit that referenced this pull request May 29, 2017
eliorivero pushed a commit that referenced this pull request May 30, 2017
* Changelog: first pass at a changelog for 5.0

* Changelog: delete 4.9 testing list.

* Changelog: update minimum WP version to match ver. in jetpack.php

Fixes #7158

* Changelog: add #6051

* Changelog: add #6753

* Changelog: add #6928

* Changelog: add #6964

* Changelog: add #7014

* Changelog: add #7057

* Changelog: add #7060

* Changelog: add #7068

* Changelog: add #7070

* Changelog: add #7072

* Changelog: add #7071

* Changelog: add release date and post shortlink.

* Changelog: add #7094

* Changelog: add #7100

* Changelog: add #7108

* Changelog: add #7113

* Changelog: add #7123

* Changelog: add #7135

* Changelog: add #7143

* Changelog: add #7151

* Changelog: add #6996

* Changelog: add #7105

* Changelog: add #7132

* Changelog: add #7166

* Changelog: fix typo in 4.9 changelog.

* Changelog: remove older releases' changelogs.

@see p1HpG7-42e-p2

* Changelog: add #7090

* Changelog: add #7095

* Changelog: add #7112

* Changelog: add #7115

* Changelog: add #7122

* Changelog: add #7137

* Changelog: add #7138

* Changelog: add #7140

* Changelog: add #7154

* Changelog: add ##7155

* Changelog: add #7163

* Changelog: add #7167

* Changelog: add #7171

* Changelog: add #7180

* Changelog: add #7181

* Changelog: add #7183

* Changelog: add #7184

* Changelog: add #7189

* Changelog: add #7191

* Changelog: add #7193

* Changelog: add #7198

* Changelog: add #7200

* Changelog: add #7209

* Changelog: add #7212

* Testing list: add instructions for #7115

* Changelog: add #7188

* Changelog: add #7205

* Changelog: add #7225

* Changelog: add #6872

* Changelog: add #7107

* Changelog: add #7118

* Changelog: add #7142

* Changelog: add #7170

* Changelog: add #7210

* Changelog: add #7218

* Changelog: add #7232

* Changelog: add #7211

* Changelog: add #7213

* Changelog: add #7229

* Changelog: add #7230

* Changelog: add #7214

* Draft changelog for 5.0

* Changelog updates: 2nd pass at a clearer changelog.

- Fix typos.
- Use consistent tense and tone across all changelog.
- Remove unclear items.

* Changelog: add #7026

* Changelog: add #7058

* Changelog: add #7125

* Changelog: add #7249

* Changelog: add #7185

* add mentions of image widget migration

* Changelog: add info about new output for CLI command.

* Changelog: add WP version number matching the new Image Widget.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants