Skip to content

Create Identifier type. - #146

Merged
marcuslinke merged 2 commits into
docker-java:masterfrom
magnayn:pr5
Mar 6, 2015
Merged

Create Identifier type.#146
marcuslinke merged 2 commits into
docker-java:masterfrom
magnayn:pr5

Conversation

@magnayn

@magnayn magnayn commented Feb 6, 2015

Copy link
Copy Markdown
Contributor

docker 'Identifiers' are easily to get wrong, as everything is a string and APIs that just take 'string' are therefore prone to error.

Users are used to tags looking like the following
tag
tag:label
host/tag
host/tag:label
host:port/tag
host:port/tag:label

Create a class to parse this into the 'repository' side and the (optional) tag side.

@marcuslinke

Copy link
Copy Markdown
Contributor

@magnayn Shouldn't we make use of Identifier in every image command and remove appropriate String versions? WDYT? The next release (1.0.0) is will break the API anyway so it would be a good time for it.

@magnayn

magnayn commented Feb 12, 2015

Copy link
Copy Markdown
Contributor Author

You could - IME Identifiers often cause mistakes in clients -- I just didn't want the scope of this PR to go changing APIs that may have some pain for people to change..

@magnayn

magnayn commented Feb 23, 2015

Copy link
Copy Markdown
Contributor Author

Rebased because of merge conflict

@marcuslinke

Copy link
Copy Markdown
Contributor

Could you add some basic tests for Repository and Identifier before I merge please. I think this would be helpful for documentation purposes. Thanks in advance!

magnayn added 2 commits March 4, 2015 09:16
at various points (e.g: 10.0.0.1:8000/mybuild:1234)

Signed-off-by: Nigel Magnay <nigel.magnay@gmail.com>
Signed-off-by: Nigel Magnay <nigel.magnay@gmail.com>
@magnayn

magnayn commented Mar 4, 2015

Copy link
Copy Markdown
Contributor Author

added some tests

@marcuslinke
marcuslinke merged commit 81b30b2 into docker-java:master Mar 6, 2015
@marcuslinke

Copy link
Copy Markdown
Contributor

@magnayn Thanks!

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