Skip to content

runtime: Deduplicate phrasing in state attribute definitions #145

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

Closed

Conversation

wking
Copy link
Contributor

@wking wking commented Sep 3, 2015

Replace:

$ENTRY ($TYPE) $ENTRY is the ...

with:

$ENTRY ($TYPE) The ...

It's more concise and avoids the need to argue over Pid vs PID ;).

This is my first commit from crosbymichael#1, rebased to pickup
the version addition that landed while #87 was in-flight.

@laijs
Copy link
Contributor

laijs commented Sep 7, 2015

LGTM

@philips
Copy link
Contributor

philips commented Sep 9, 2015

I like the style that reads like a sentence. For example:

-* root (string) is the path to the container's bundle directory.

@wking wking force-pushed the dedup-state-attribute-definitions branch from 81faf1e to 49f1bbd Compare September 9, 2015 18:23
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Sep 9, 2015
Replace:

  $ENTRY ($TYPE) $ENTRY is the ...

with:

  $ENTRY ($TYPE) is the ...

It's more concise and avoids the need to argue over Pid vs PID ;).
And "reads like a sentence" is an easy style to check and remember
[1].

[1]: opencontainers#145 (comment)

Signed-off-by: W. Trevor King <[email protected]>
@wking
Copy link
Contributor Author

wking commented Sep 9, 2015

On Wed, Sep 09, 2015 at 09:47:49AM -0700, Brandon Philips wrote:

I like the style that reads like a sentence. For example:

-* root (string) is the path to the container's bundle directory.

Sounds good to me. Fixed and rebased onto the current master with
81faf1e49f1bbd.

@vbatts
Copy link
Member

vbatts commented Sep 10, 2015

@wking looks like a funky rebase that is picking up other commits?

@wking
Copy link
Contributor Author

wking commented Sep 10, 2015

On Wed, Sep 09, 2015 at 07:56:17PM -0700, Vincent Batts wrote:

@wking looks like a funky rebase that is picking up other commits?

I may have been bitten by the actions behind this IRC comment:

14:48 < vbatts> i just used github's API to force push master from a
commit that is only in their reflog

And here's the history I've seen for the master branch:

$ git reflog origin/master
9a8748c refs/remotes/origin/master@{0}: fetch --append origin: fast-forward
fbac038 refs/remotes/origin/master@{1}: fetch --append origin: forced-update
fd0ff56 refs/remotes/origin/master@{2}: fetch --append origin: fast-forward
931c6a9 refs/remotes/origin/master@{3}: fetch --append origin: fast-forward

Here's a graph of what I see:

$ git log --graph --topo-order --oneline --decorate origin/master dedup-state-attribute-definitions fd0ff56

So it looks like this PR, the #163 merge, and the #159 merge were
abandoned by your force-push. I haven't gone through the IRC history
in more detail yet to understand what you were trying to accomplish,
but I'll report back once I have a clearer idea.

@wking
Copy link
Contributor Author

wking commented Sep 10, 2015

On Wed, Sep 09, 2015 at 08:25:32PM -0700, W. Trevor King wrote:

So it looks like this PR, the #163 merge, and the #159 merge were
abandoned by your force-push. I haven't gone through the IRC history
in more detail yet to understand what you were trying to accomplish,
but I'll report back once I have a clearer idea.

I haven't been able to figure out what was wrong from the IRC logs.
Here are the entries that appeared relevant:

14:23 < vbatts> crosbymichael: what commit is your origin/master at
14:23 < vbatts> before git pull
14:25 < vbatts> i think it was force pushed
14:26 < vbatts> :-|
14:27 < crosbymichael> umm
14:27 < crosbymichael> 16147
14:27 < crosbymichael> 527a3ee
14:27 < crosbymichael> that
14:27 < crosbymichael> now 26b9be2
14:28 < vbatts> it should be fbac038
14:28 < vbatts> fbac038
14:29 < vbatts> https://github.com/opencontainers/specs/pulls?q=is%3Apr+is%3Aclosed it's missing the most recent 7 merges
14:48 < vbatts> i just used github's API to force push master from a commit that is only in their reflog
14:49 < vbatts> tianon: curl -u vbatts -H "Authorization: token FARTFARTFARTS" --request PATCH https://api.github.com/repos/opencontainers/specs/git/refs/heads/master --data '{"sha": "fbac038b38ef0cc8bef62c911e658f41f98ff42d", "force": true}'
14:49 < vbatts> crosbymichael: origin is fixed now

So I'm not sure how you decided on fbac038, but I think you should
at least merge fd0ff56 into the current master. And folks should
stop force-pushing to master ;).

@wking wking force-pushed the dedup-state-attribute-definitions branch from 49f1bbd to c496947 Compare September 10, 2015 17:14
Replace:

  $ENTRY ($TYPE) $ENTRY is the ...

with:

  $ENTRY ($TYPE) is the ...

It's more concise and avoids the need to argue over Pid vs PID ;).
And "reads like a sentence" is an easy style to check and remember
[1].

[1]: opencontainers#145 (comment)

Signed-off-by: W. Trevor King <[email protected]>
@wking
Copy link
Contributor Author

wking commented Sep 10, 2015

On Wed, Sep 09, 2015 at 07:56:17PM -0700, Vincent Batts wrote:

@wking looks like a funky rebase that is picking up other commits?

With 3f62423 and 712a746 you pulled in #159 and #163. Those were the
only PRs that were part of the abandoned fd0ff56 history, so I've
rebased my commit onto origin/master and we're back down to just my
change.

@vbatts
Copy link
Member

vbatts commented Sep 10, 2015

LGTM

@wking
Copy link
Contributor Author

wking commented Oct 2, 2015

This has been rolled into #211.

@wking wking closed this Oct 2, 2015
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.

4 participants