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

Removing Link Introduction #856

Merged
merged 1 commit into from
Jul 9, 2021
Merged

Removing Link Introduction #856

merged 1 commit into from
Jul 9, 2021

Conversation

vsoch
Copy link
Contributor

@vsoch vsoch commented Jun 18, 2021

There doesn't seem to be an introduction in this file (and the link goes to itself?) so I think we probably want to remove it (or link to an introduction elsewhere?)

spec.md Outdated
@@ -42,7 +41,7 @@ Typically, these manifests may provide different implementations of the image, p

![](img/build-diagram.png)

Once built the OCI Image can then be discovered by name, downloaded, verified by hash, trusted through a signature, and unpacked into an [OCI Runtime Bundle](https://github.com/opencontainers/runtime-spec/blob/master/bundle.md).
Once built the OCI image can then be discovered by name, downloaded, verified by hash, trusted through a signature, and unpacked into an [OCI Runtime Bundle](https://github.com/opencontainers/runtime-spec/blob/master/bundle.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

I see OCI Image used throughout the spec, so I don't know if I agree with this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are also a lot of lowercase - I guess I don't see it as a proper noun so I made it lowercase. But if you are requesting to make it capital again, happy to do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All set.

There doesn't seem to be an introduction in this file (and the link goes to itself?) so I think we probably want to remove it (or link to an introduction elsewhere?)

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
Copy link
Contributor

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

@vbatts
Copy link
Member

vbatts commented Jun 22, 2021

It looks like the DCO/signoff may have an issue with a difference in the commit email and the signoff email? not sure

@vsoch
Copy link
Contributor Author

vsoch commented Jun 22, 2021

I did change both emails to be my GitHub no reply email when I left Stanford.

@vsoch
Copy link
Contributor Author

vsoch commented Jun 22, 2021

Where do you see the issue? It looks green to me!

@vbatts
Copy link
Member

vbatts commented Jun 23, 2021

https://pullapprove.com/opencontainers/image-spec/pull-request/856/requirements/
looks like a prefix of 814322+ in the commit email?

@vsoch
Copy link
Contributor Author

vsoch commented Jun 23, 2021

@vbatts GitHub holds an internal representation of that address with and without the number. It's kind of complicated but without signing with the number it doesn't link to me, so the gpg key actually has all of those emails (and my original stanford) so I'm not sure why one is explicitly chosen. I wrote more about it here: https://vsoch.github.io/2021/signing-with-myself/.

@vsoch
Copy link
Contributor Author

vsoch commented Jun 23, 2021

I did create the original commit in the GitHub interface and then pulled and rebased, so that might be why the two are different? I can close here and re-do again if you want... this seems like an extraordinary amount of work to just remove one line! I'm not sure why it's always so hard to contribute to these repos :/

@vbatts
Copy link
Member

vbatts commented Jun 23, 2021

Fair. And we have 2 DCO checks, and only one fail? 🤔

Copy link
Member

@vbatts vbatts left a comment

Choose a reason for hiding this comment

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

🌃

@vbatts
Copy link
Member

vbatts commented Jun 23, 2021

@caniszczyk do we need the pullapprove DCO check also, since it's being a little strict?

@vbatts vbatts merged commit 5ced465 into opencontainers:main Jul 9, 2021
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.

5 participants