-
Notifications
You must be signed in to change notification settings - Fork 26
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
Correct README to reflect the defork from mozilla/go-cose #69
Conversation
8c1dfc4
to
7d7f0fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple of grammar fixes
README.md
Outdated
@@ -10,9 +10,9 @@ A golang library for the [COSE specification][cose-spec] | |||
|
|||
**Current Release**: [go-cose alpha 1][release-alpha-1] | |||
|
|||
The project was *initially* forked from the upstream [mozilla/go-cose][mozilla-go-cose] project, however it was agreed between the veraison and mozilla maintainers to retire mozilla/go-cose project and focus on [veraison/go-cose][veraison-go-cose] as the active project. | |||
The project was *initially* forked from the upstream [mozilla-services/go-cose][mozilla-go-cose] project, however it was agreed between the Veraison and Mozilla maintainers to retire mozilla-services/go-cose project and focus on [veraison/go-cose][veraison-go-cose] as the active project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer using the active phrasing (i.e., as it was before) to the passive construction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing wrong in introducing with a brief background of the initial state (earlier) and then thanking the original contributors (in present) and then moving on. In my view it flows well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what I am referring to is the change from the original:
the veraison and mozilla maintainers have agreed to [...]
to
it was agreed between the Veraison and Mozilla maintainers to [...]
(the former is better.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that was correct when the action was in progress, like I have agreed to hand over XYZ to you.
But once the handover happened that should also go to past tense, like It was agreed between UK and EU to part ways back in 2019.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it's not that the agreement has vanished. Or so I hope :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully this is viewed far better than brexit :)
@SteveLasker & @thomas-fossati Could you please approve the PR. Seems to be in pretty good shape. We need to make a release now once this is merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Looks great. @thomas-fossati, if you could add the other LGTM, we could merge. |
Thanks @SteveLasker & @thomas-fossati for adding your comments and review. |
No description provided.