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

[Style] Remove "should" not related to conformance #536

Closed
wants to merge 3 commits into from

Conversation

RobDolinMS
Copy link
Collaborator

@RobDolinMS RobDolinMS commented Aug 22, 2016

The style.md file includes the word "should" which may unintentionally be confused with "SHOULD" as used to identify a conformance/certification requirement.

This pull request removes the word "should" so there is not confusion

Signed-off-by: Rob Dolin [email protected]

The style.md file includes the word "should" which may unintentionally be confused with "SHOULD" as used to identify a conformance/certification requirement.  

This pull request removes the word "should" so there is not confusion

Signed-off-by: Rob Dolin <[email protected]>
@@ -2,21 +2,21 @@

## One sentence per line

To keep consistency throughout the Markdown files in the Open Container spec all files should be formatted one sentence per line.
To keep consistency throughout the Markdown files in the Open Container spec format all files one sentence per line.
Copy link
Contributor

Choose a reason for hiding this comment

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

“spec format” → “spec, format”

@wking
Copy link
Contributor

wking commented Aug 22, 2016

On Mon, Aug 22, 2016 at 12:28:23PM -0700, Rob Dolin (MSFT) wrote:

The style.md file includes the word "should" which may
unintentionally be confused with "SHOULD" as used to identify a
conformance/certification requirement.

ALL CAPS are pretty striking. And style.md is fairly clearly
targetting runtime-spec authors, although it will explicitly not be
included in the protocol listing that's in flight with #527. So I
don't think these changes are particularly critical, but they
shouldn't hurt either ;).

@wking
Copy link
Contributor

wking commented Aug 22, 2016

On Mon, Aug 22, 2016 at 12:28:23PM -0700, Rob Dolin (MSFT) wrote:

  • [Style] Remove "should" not realted to conformance

“realted” → “related”

@RobDolinMS RobDolinMS changed the title [Style] Remove "should" not realted to conformance [Style] Remove "should" not related to conformance Aug 31, 2016
@RobDolinMS
Copy link
Collaborator Author

I believe all of the issues @wking raised have now been addressed (including my typo in the PR title)

@wking
Copy link
Contributor

wking commented Aug 31, 2016 via email

@vbatts
Copy link
Member

vbatts commented Sep 1, 2016

LGTM

Approved with PullApprove

@crosbymichael
Copy link
Member

There is still a signed off missing on a commit, it would be better to just squash them all.

@tianon
Copy link
Member

tianon commented Sep 8, 2016

LGTM, and appears to be all-signed-off to me (and Travis agrees) 👍

Approved with PullApprove

@tianon
Copy link
Member

tianon commented Sep 8, 2016

It's pullapprove which disagrees! What!

@tianon
Copy link
Member

tianon commented Sep 8, 2016

Not seeing any issues here, pullapprove...

image

@caniszczyk
Copy link
Contributor

Interesting, I wonder what emails @RobDolinMS has associated with this github account.

Is it [email protected]?

@tianon
Copy link
Member

tianon commented Nov 4, 2016

@caniszczyk did we ever figure out what's causing @RobDolinMS to be shafted by PullApprove? 😢

@wking
Copy link
Contributor

wking commented Nov 4, 2016

On Fri, Nov 04, 2016 at 03:56:53PM -0700, Tianon Gravi wrote:

@caniszczyk did we ever figure out what's causing @RobDolinMS to be
shafted by PullApprove? 😢

My random guess is that he has (MSFT) in his author field but not in
his s-o-b:

$ git show origin/pr/536 | head -n 7
commit 6d500fb
Author: Rob Dolin (MSFT) [email protected]
Date: Wed Aug 31 14:29:31 2016 -0700

  [Style] Update sentence about example headers

  Signed-off-by: Rob Dolin <[email protected]>

@mrunalp
Copy link
Contributor

mrunalp commented Jan 11, 2017

@RobDolinMS could you try matching the Author and Signed-off-by name?

@crosbymichael
Copy link
Member

@RobDolinMS when you have time please rebase your PRs and make sure your sign off is correct. Feel free to reopen when they have been updated.

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.

7 participants