Skip to content

Conversation

@thaJeztah
Copy link
Member

The uuid package in distribution was created as a utility for the distribution project itself, to cut down external dependencies (see 1).

For compose, this has the reverse effect, as it now brings all the dependencies of the distribution module with it.

This patch switches to the uuid generation to crypto/rand to produce a random id. I was considering using a different uuid implementation, or docker's "stringid.GenerateRandomID", but all of those are doing more than needed, so keep it simple.

Currently, this change has little effect, because compose also uses the distribution module for other purposes, but the distribution project is in the process of moving the "reference" package to a separate module, in which case we don't want to depend on the distribution module only for the uuid package.

What I did

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did

The uuid package in distribution was created as a utility for the distribution
project itself, to cut down external dependencies (see [1][1]).

For compose, this has the reverse effect, as it now brings all the dependencies
of the distribution module with it.

This patch switches to the uuid generation to crypto/rand to produce a random
id. I was considering using a different uuid implementation, or docker's
"stringid.GenerateRandomID", but all of those are doing more than needed,
so keep it simple.

Currently, this change has little effect, because compose also uses the
distribution module for other purposes, but the distribution project is
in the process of moving the "reference" package to a separate module,
in which case we don't want to depend on the distribution module only for
the uuid package.

[1]: distribution/distribution@36e34a5

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov
Copy link

codecov bot commented Aug 31, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.05% ⚠️

Comparison is base (d6f842b) 58.32% compared to head (8caa6f1) 58.27%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10953      +/-   ##
==========================================
- Coverage   58.32%   58.27%   -0.05%     
==========================================
  Files         123      123              
  Lines       10864    10866       +2     
==========================================
- Hits         6336     6332       -4     
- Misses       3909     3914       +5     
- Partials      619      620       +1     
Files Changed Coverage Δ
pkg/api/dryrunclient.go 0.00% <0.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ndeloof ndeloof requested review from a team, StefanScherer, glours, laurazard, milas, nicksieger and ulyssessouza and removed request for a team August 31, 2023 14:04
@milas milas merged commit d0dfb84 into docker:main Aug 31, 2023
@milas milas added the dependencies Pull requests that update a dependency file label Aug 31, 2023
@thaJeztah thaJeztah deleted the drop_uuid branch August 31, 2023 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants