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

Adding questions/answers to spec #189

Closed
wants to merge 3 commits into from
Closed

Adding questions/answers to spec #189

wants to merge 3 commits into from

Conversation

vsoch
Copy link
Contributor

@vsoch vsoch commented Sep 24, 2020

During start of implementation for the distribution spec, I had a lot of basic questions about different scenarios and modeling, and (even if the spec doesn't have a definitive answer) I think it would be helpful to include more detail to help the reader/user. The pull request here will cover most of the discussion in #184, adding questions -> answers along with the spec details to help guide the reader. Please feel free to make suggests to wording or content!

Signed-off-by: vsoch vsochat@stanford.edu

spec.md Outdated
@@ -133,7 +133,44 @@ If the blob is not found in the registry, the response code MUST be `404 Not Fou

#### Push
Pushing an image works in the opposite order as a pull: the blobs making up the layers are pushed first, and the
manifest last.
manifest last. A useful diagram is provided [here](https://github.com/google/go-containerregistry/tree/d7f8d06c87ed209507dd5f2d723267fe35b38a9f/pkg/v1/remote#anatomy-of-an-image-upload).
Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't want to just link out to another repo but find it useful, the source for this is here: https://github.com/google/go-containerregistry/blob/master/images/dot/upload.dot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it would be redundant to re-host it, and what if the other version changes but ours does not? What do others think?

@jdolitsky
Copy link
Member

@vsoch this is good stuff. Do you think it makes sense in its own FAQ section at bottom? Regarding links etc., please see #188 where we are adding an appendix.

Also - can you fix the travis build?

0.06s$ TRAVIS_COMMIT_RANGE="${TRAVIS_COMMIT_RANGE/.../..}" make .gitvalidation
git-validation -q -run DCO,short-subject,dangling-whitespace
 b602e8f - FAIL - has whitespace errors. See `git show --check b602e8fce277faeb0b384f8de117f3090249f473`.
1 commits to fix
Makefile:54: recipe for target '.gitvalidation' failed
make: *** [.gitvalidation] Error 1

Copy link
Member

@jdolitsky jdolitsky left a comment

Choose a reason for hiding this comment

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

Good content, but majority should be placed into a top-level ## F.A.Q. section right before the API section. For links to diagrams, we can add them to the appendix

spec.md Outdated Show resolved Hide resolved
@vsoch
Copy link
Contributor Author

vsoch commented Sep 25, 2020

@jdolitsky @jonjohnsonjr that's a good idea to have them in FAQ - but is there a way we can link them back to where they are relevant? We'd want the person reading / implementing to read points when they are relevant, otherwise they might be missed entirely. Maybe a link from the FAQ back to the section, so for the section, a small list of relevant FAQ at the end of each?

@vsoch
Copy link
Contributor Author

vsoch commented Sep 25, 2020

okay I've moved all the question snippets into it's own FAQ section - it's not ideal in that it becomes unlinked from the relevant sections, but hopefully having it right before the specification sections (push, etc.) the user will at least glance over it and know to come back if there is a question.

@adamwg
Copy link

adamwg commented Oct 7, 2020

I wonder whether this content should move to a separate "advice for implementors" or "implementation guide" document. That would accomplish a few goals:

  1. Keep the spec document small and focused on requirements.
  2. Allow the spec to link to sections of the implementation guide (or vice-versa), which would help with @vsoch's point about the two being unlinked.
  3. Make it easy for the implementation guide to grow/evolve separately from the spec, which seems appropriate.

@vsoch
Copy link
Contributor Author

vsoch commented Oct 7, 2020

@adamwg I'm happy to do that, just let me know what you want. Also, it looks like there was a merge so now there is a merge conflict. I can attempt to resolve but I may not be familiar enough with what should be there to do it correctly.

@vsoch
Copy link
Contributor Author

vsoch commented Oct 7, 2020

okay looks like I was able to do it! Hopefully correctly, haha.

@jdolitsky
Copy link
Member

+1 to Adam's comment. This document could (and should) grow pretty large. Howabout an faq.md ?

@caniszczyk
Copy link
Contributor

caniszczyk commented Oct 8, 2020 via email

@jdolitsky
Copy link
Member

That works as well. image-spec has something similar: https://github.com/opencontainers/image-spec

@vsoch
Copy link
Contributor Author

vsoch commented Oct 8, 2020

Moved.

@jdolitsky
Copy link
Member

@vsoch awesome. Can you fix the signoff error?

@vsoch
Copy link
Contributor Author

vsoch commented Oct 8, 2020

How exactly? I merged master into here from the interface, and here is the git history. Are you saying I have to start from scratch? Why is it so hard to contribute here? :(

Author: vsoch <vsochat@stanford.edu>
Date:   Thu Oct 8 09:30:35 2020 -0600

    moving questions to faq in readme
    
    Signed-off-by: vsoch <vsochat@stanford.edu>

commit c2077da8d4773ee2ac20562c49639b89f83e0833
Merge: 4f01bdd db37dc2
Author: Vanessasaurus <vsochat@stanford.edu>
Date:   Wed Oct 7 16:09:05 2020 -0600

    Merge branch 'master' into add/spec-details

Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch
Copy link
Contributor Author

vsoch commented Oct 8, 2020

I started from scratch and updated the files, I hope that worked. :/

@jdolitsky
Copy link
Member

@vsoch I understand the pain. Looks like it worked

@jdolitsky jdolitsky added this to the v1.0.0-rc2 milestone Nov 2, 2020
@jdolitsky
Copy link
Member

@vsoch - apologies, but there was some conversation about this - can we move this into an faq.md in the root of the repo? There are a bunch more things we would like to add there

@vsoch
Copy link
Contributor Author

vsoch commented Dec 1, 2020

Sure thing! Let me pull and make the changes.

Signed-off-by: vsoch <vsochat@stanford.edu>
@jdolitsky
Copy link
Member

@vsoch - looks good, can you fix the sign-off issue? One of the commits must be missing it

@vsoch
Copy link
Contributor Author

vsoch commented Dec 7, 2020

All of the commits are signed (see verified in the Commits tab). What am I missing?

@jdolitsky
Copy link
Member

@vsoch - its the merge commit: c83bd62

@vsoch
Copy link
Contributor Author

vsoch commented Dec 9, 2020

I had to update from master (there was a merge conflict), I did that on GitHub automatically.

@vsoch vsoch closed this Dec 12, 2020
jdolitsky pushed a commit that referenced this pull request Dec 15, 2020
Signed-off-by: vsoch <vsochat@stanford.edu>
@jdolitsky jdolitsky mentioned this pull request Mar 24, 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.

None yet

5 participants