Skip to content

Create Identifier type.#146

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

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

Conversation

@magnayn
Copy link
Copy Markdown
Contributor

@magnayn magnayn commented Feb 6, 2015

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
Copy link
Copy Markdown
Contributor Author

magnayn commented Feb 12, 2015

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
Copy link
Copy Markdown
Contributor Author

magnayn commented Feb 23, 2015

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 <[email protected]>
@magnayn
Copy link
Copy Markdown
Contributor Author

magnayn commented Mar 4, 2015

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