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

Addressing multiple issues for v10.0.0. Fixes #126, #136, #171, #174. #503

Merged
merged 4 commits into from
Jul 14, 2021

Conversation

sa-bpelakh
Copy link
Collaborator

  • Renamed decimalValue to numericValue
  • Changes to Person, including name and hasBirthDate
  • Refactored network connections, renaming properties and tightening up their definition
  • Clarified definition of ContemporaneousEvent

Tagging @Jamie-SA for numericValue, the usual suspects for the rest.

- Renamed decimalValue to numericValue
- Changes to Person, including name and hasBirthDate
- Refactored network connections, renaming properties and tightening up their definition
- Clarified definition of ContemporaneousEvent
Copy link
Collaborator

@rjyounes rjyounes left a comment

Choose a reason for hiding this comment

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

I've proposed some annotation changes, although they aren't included in the decisions made on the issues, because I always scan these and make corrections when I'm working on a term. I find the current network-related ones quite confusing; and in fact I believe were the source of our confusion about how these terms should be used.

Everything else looks good.

Copy link
Contributor

@Jamie-SA Jamie-SA left a comment

Choose a reason for hiding this comment

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

gist:numericValue looks good.
One comment below that can be ignored.

@sa-bpelakh sa-bpelakh requested a review from rjyounes July 5, 2021 17:29
Copy link
Collaborator

@rjyounes rjyounes left a comment

Choose a reason for hiding this comment

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

Everything looks good except for one missing period. I do not need to re-review after it's added.

Copy link
Contributor

@uscholdm uscholdm left a comment

Choose a reason for hiding this comment

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

The current versions (on github, but not showing up on this web page) look fine, but for one small thing. Replace Unlike the superproperty, links, with Unlike its superproperty, links,

If you strongly object with having links mentioned, I won't be that bothered. I like it because it saves having to look at the code, which is kind of the point of a definition.

@sa-bpelakh sa-bpelakh requested a review from uscholdm July 14, 2021 19:29
@rjyounes
Copy link
Collaborator

@sa-bpelakh Need to resolve conflicts in release notes.

@sa-bpelakh sa-bpelakh merged commit 5b70eba into develop Jul 14, 2021
@sa-bpelakh sa-bpelakh deleted the feature/v10.0.0-cleanup-126-136-171-174 branch July 14, 2021 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants