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

Breakup common package #24

Merged
merged 5 commits into from Jan 6, 2015
Merged

Conversation

stevvooe
Copy link
Collaborator

@stevvooe stevvooe commented Jan 6, 2015

Catchall common packages are generally an indication of bad organization, so we are getting rid of it while it is small. Mostly, types were moved to other, more relevant packages or a new package was created. Please see the individual commits for details.

This addresses items discussed in #6.

As part of the efforts to break up the common package before disaster strikes,
a new collections package has been created. More may belong there but for now,
it only includes an implementation of StringSet.

Signed-off-by: Stephen J Day <stephen.day@docker.com>
In preparation for removing the common package, the tarsum utilities are being
moved to the more relevant digest package. This functionality will probably go
away in the future, but it's maintained here for the time being.

Signed-off-by: Stephen J Day <stephen.day@docker.com>
Because the repository name definitions are part of the v2 specification, they
have been moved out of the common package. This is part of the effort to break
up the common package into more sensible components.

Signed-off-by: Stephen J Day <stephen.day@docker.com>
Since the common package no longer exists, the testutil package is being moved
up to the root. Ideally, we don't have large omnibus packages, like testutil,
but we can fix that in another refactoring round.

Signed-off-by: Stephen J Day <stephen.day@docker.com>
@stevvooe
Copy link
Collaborator Author

stevvooe commented Jan 6, 2015

@stevvooe stevvooe added this to the Registry/Beta milestone Jan 6, 2015
@stevvooe stevvooe mentioned this pull request Jan 6, 2015
4 tasks
@dmp42
Copy link
Contributor

dmp42 commented Jan 6, 2015

LGTM

@icecrime
Copy link

icecrime commented Jan 6, 2015

Are we creating a collections package only for type StringSet map[string]struct{}?

@stevvooe
Copy link
Collaborator Author

stevvooe commented Jan 6, 2015

@icecrime Yes. I'm not tied to the concept of a collections package. Hopefully, this avoids a million definitions of stringsets where they are used. Alternatively, we can just move this back into auth/token and make it private.

Let me know what you prefer.

@icecrime
Copy link

icecrime commented Jan 6, 2015

You know better, but if it's only used in auth/token let's just have it there.

The exported StringSet type is not necessary for the current use case of
validating issues and audiences. The exported fields on VerifyOptions have been
changed to require string slices. The collections package has been removed and
the StringSet has been moved to the token package, where it is used.

Signed-off-by: Stephen J Day <stephen.day@docker.com>
@stevvooe
Copy link
Collaborator Author

stevvooe commented Jan 6, 2015

@icecrime Fixed. ;)

@BrianBland
Copy link
Contributor

LGTM

stevvooe added a commit that referenced this pull request Jan 6, 2015
@stevvooe stevvooe merged commit fdea60a into distribution:master Jan 6, 2015
@stevvooe stevvooe deleted the breakup-common branch January 6, 2015 18:08
@stevvooe stevvooe modified the milestones: Registry/2.0.0-beta, Registry/2.0 Mar 31, 2015
openshift-publish-robot referenced this pull request in openshift/docker-distribution May 21, 2018
bump(*)

Image-registry-commit: 8f7ef225cf4dada45f2e72ea729c50bb62324539
openshift-publish-robot referenced this pull request in openshift/docker-distribution Jun 18, 2018
bump(*)

Image-registry-commit: 8f7ef225cf4dada45f2e72ea729c50bb62324539
dmage pushed a commit to dmage/distribution that referenced this pull request Feb 23, 2022
UPSTREAM: docker/distribution: 3057: allow for optional S3 debugging -- closes issue distribution#2043
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants