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

Fixed some typos, use of RFC 2119 and links #78

Merged
merged 1 commit into from
Jun 5, 2019

Conversation

cyenyxe
Copy link

@cyenyxe cyenyxe commented Jun 5, 2019

Some typos fixed, and added use of RFC 2119 in a couple of places. The link to the Interbase Interval Tests is now fixed, but it points to an archived file; it should be decided whether to keep it.

In addition to these small changes, I have some suggestions or questions from the terms and model index page:

  • Receiving systems SHOULD NOT persist Ids from remote sources. Instead, Identifiers (below) SHOULD be used for communication between systems. -> MUST (NOT) instead of SHOULD (NOT)?
  • Use of 'MUST' instead of 'SHALL' could remove the ambiguity that word has (can be a suggestion or a command)

Copy link
Member

@ahwagner ahwagner left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@ahwagner ahwagner merged commit 18dfc89 into ga4gh:master Jun 5, 2019
@cyenyxe
Copy link
Author

cyenyxe commented Jun 6, 2019

Fantastic! What do you think about the couple of questions raised in my first comment, should I address them or leave them be?

@ahwagner
Copy link
Member

ahwagner commented Jun 6, 2019

I'm not sure on bullet 1; there may have been a reason this was written as SHOULD NOT; perhaps there may be implementations that wish to for reasons we haven't considered? Anyway, I agree with you, but we should get feedback from @rrfreimuth @larrybabb and/or @reece first.

I agree with you on point 2, please replace any instances you find (though I'm not sure there are any?)

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