spegel-org / spegel

Stateless cluster local OCI registry mirror.
MIT License
1.3k stars 70 forks source link

Append wrongly named / Missing docs #617

Open PrivatePuffin opened 1 month ago

PrivatePuffin commented 1 month ago

Spegel version

All

Kubernetes distribution

Irrelevant

Kubernetes version

Irrelevant

CNI

Irrelevant

Describe the bug

The Append function for mirrors seems to be incorrectly named. I've gone over the tests and seen what it actually does.

Technically its a prepend, add before something else, not an append

As breaking the naming now is annoying, this should at least be very clearly documented.

phillebaba commented 3 weeks ago

This is true. Reading the docs for the value it is a bit confusing.

When true existing mirror configuration will be appended to instead of replaced.

What I tried to communicate is that existing mirrors are appended to the the mirror configuration. Maybe appendExistingMirrors would be a better name.

PrivatePuffin commented 3 weeks ago

This is true. Reading the docs for the value it is a bit confusing.

When true existing mirror configuration will be appended to instead of replaced.

Its mostly a docs issue x will be appended, means that Y is added below X. In this case "existing mirror configuration" will be appended, means that "spegel" is added below "existing mirror configuration"

So the issue is mostly the fact that this sentence is wrong. Keeping the same kind of sentence, the correct way to write this would be:

When true existing mirror configuration will be appended to the spegel mirror configuration