Skip to content

Conversation

@wking
Copy link
Contributor

@wking wking commented Feb 16, 2017

Start distinguishing between reader (when you only need to Get CAS blobs) and walker (when you might also need to access mutable refs). This lays the ground work for something like #5, but avoids the contentious invention of new interfaces by sticking to interfaces which have already landed in master. Also removes a number of redundant Get re-implementations.

Make it clear that this only needs the CAS-oriented Get and not the
full walker.

Signed-off-by: W. Trevor King <[email protected]>
Avoid locally implementing Get.

Signed-off-by: W. Trevor King <[email protected]>
Avoid locally implementing Get.

Signed-off-by: W. Trevor King <[email protected]>
@wking wking force-pushed the consistent-reader-usage branch from 2e92353 to e815bc7 Compare February 16, 2017 11:27
Both of these calls are to descriptor.validate(), which only needs a
reader.

Signed-off-by: W. Trevor King <[email protected]>
This is only getting CAS blobs, so we only need a reader.  And there's
no need to locally implement Get.

Signed-off-by: W. Trevor King <[email protected]>
Now that we've ported manifest.unpack over to reader, we can replace
the more-complicated walker implementation (which also provides for
mutable-ref access) with the more-basic reader (which only interacts
with CAS blobs).

Signed-off-by: W. Trevor King <[email protected]>
@wking wking force-pushed the consistent-reader-usage branch from e815bc7 to 27c98e0 Compare February 16, 2017 11:30
@zhouhao3
Copy link

zhouhao3 commented Nov 1, 2017

@wking This PR and #143 have some similar effects. Can you rebase this PR? And then we'll talk about the desirability of these.

@wking
Copy link
Contributor Author

wking commented Nov 1, 2017

It looks like reader was removed in #136, so there's no longer a way to make this distinction with the interfaces defined by the current image-tools.

@wking wking closed this Nov 1, 2017
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.

2 participants