Skip to content

Represent Cypher DateTime with a single type #352

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

Merged
merged 1 commit into from
Apr 12, 2018

Conversation

lutovich
Copy link
Contributor

DateTime contains time zone information. It might either be an offset in seconds (that corresponds to ±HH:MM) or the time zone id (like 'Europe/London'). Previously driver exposed DateTime as two different types: DateTimeWithZoneOffset and DateTimeWithZoneId.

This PR combines them into a single type:

class DateTime {
  year: Integer|number,
  month: Integer|number,
  day: Integer|number,
  hour: Integer|number,
  minute: Integer|number,
  second: Integer|number,
  nanosecond: Integer|number,
  timeZoneOffsetSeconds: Integer|number,
  timeZoneId: string
}

where either timeZoneOffsetSeconds or timeZoneId is defined. Such single type better corresponds to the Cypher DateTime type and feels more idiomatic to JavaScript. Shape of objects (set of their properties) feels more important than the class/type.

Also renamed Time.offsetSeconds to Time.timeZoneOffsetSeconds to better represent the meaning of the property.

@lutovich lutovich requested a review from ali-ince April 11, 2018 16:48
DateTime contains time zone information. It might either be an offset
in seconds (that corresponds to ±HH:MM) or the time zone id
(like 'Europe/London'). Previously driver exposed DateTime as two
different types: `DateTimeWithZoneOffset` and `DateTimeWithZoneId`.

This commit combines them into a single type:

```
class DateTime {
  year: Integer|number,
  month: Integer|number,
  day: Integer|number,
  hour: Integer|number,
  minute: Integer|number,
  second: Integer|number,
  nanosecond: Integer|number,
  timeZoneOffsetSeconds: Integer|number,
  timeZoneId: string
}
```

where either `timeZoneOffsetSeconds` or `timeZoneId` is defined. Such
single type better corresponds to the Cypher DateTime type and feels
more idiomatic to JavaScript. Shape of objects (set of their properties)
feels more important than the class/type.

Also renamed `Time.offsetSeconds` to `Time.timeZoneOffsetSeconds` to
better represent the meaning of the property.
@lutovich lutovich force-pushed the 1.6-single-DateTime-type branch from 959ff20 to 51b8b91 Compare April 12, 2018 09:57
Copy link
Contributor

@ali-ince ali-ince left a comment

Choose a reason for hiding this comment

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

👍

@ali-ince ali-ince merged commit a558fcf into neo4j:1.6 Apr 12, 2018
@lutovich lutovich deleted the 1.6-single-DateTime-type branch April 12, 2018 12:37
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.

2 participants