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

Fix incorrect documentation for schedule time format #232

Merged
merged 4 commits into from
Jun 22, 2016
Merged

Conversation

dmurray-r7
Copy link
Contributor

No description provided.

@dmurray-r7
Copy link
Contributor Author

@gschneider-r7 @sgreen-r7 cleared up some of the comments around schedule start time format and added in some notes around timezones.

@@ -154,11 +149,12 @@ class Schedule < APIObject
attr_accessor :day
attr_accessor :occurrence
attr_accessor :start_month
# Timezone cannot be set, always console timezone. If console timezone is not supported defaults to utc.
attr_accessor :timezone
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this comment only apply on write, or also read?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll reword this, timezone can be read back from the site schedule. But when creating a schedule adding a timezone will do nothing.

@@ -154,11 +149,12 @@ class Schedule < APIObject
attr_accessor :day
attr_accessor :occurrence
attr_accessor :start_month
# Timezone cannot be set on schedule create, it is set to console timezone. If console timezone is not supported it defaults to utc.

Choose a reason for hiding this comment

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

Line is too long. [136/120]

# The amount of time, in minutes, to allow execution before stopping.
attr_accessor :max_duration
# The date after which the schedule is disabled, in ISO 8601 format.
attr_accessor :not_valid_after

# TODO: Remove this unused attribute
attr_accessor :incremental
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine to remove without needing to bump major version. @sgreen-r7 agree?

Copy link
Contributor

@sgreen-r7 sgreen-r7 Jun 15, 2016

Choose a reason for hiding this comment

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

No. 😃
Also, if we're removing that attr, then we should remove the other places in the code that are calling it. schedule.incremental = (xml.attributes['incremental'] && xml.attributes['incremental'] == '1'))

@dmurray-r7 @gschneider-r7

@sgreen-r7
Copy link
Contributor

Looks good to me @dmurray-r7
@gschneider-r7 anything else?

And we will do a major version bump now for sure 😄

@dmurray-r7
Copy link
Contributor Author

@sgreen-r7 @gschneider-r7 Happy enough for this to be merged in?

@sgreen-r7
Copy link
Contributor

I'm fine with it, @gschneider-r7 ?

@gschneider-r7
Copy link
Contributor

🚢 it

@gschneider-r7 gschneider-r7 merged commit 1859c4b into master Jun 22, 2016
@gschneider-r7 gschneider-r7 deleted the fix_docs branch June 22, 2016 17:45
This pull request was closed.
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