respect arch param when sort=semver in [DockerVersion] badge

[chris48s]

Refs https://github.com/badges/shields/discussions/10904

This is a bit of a "remove spacebar heating" change

I think my reading here is that this change fundamentally brings the code into line with the documented behaviour, so we could consider it a non-breaking change.

That said, this probably is going to change the result of this badge for some projects in the wild which rely on the fact the semver variant ignores architectures (because it assumes projects follow the more normal pattern where you push multiple architectures to a single version tag).

As such, this is a bit of a debatable one. I can see that we could equally choose to not merge this to preserve backwards compatibility and document more clearly the existing behaviour.

[github-actions[bot]]

	Warnings
:warning:	This PR modified service code for docker but not its test code. That's okay so long as it's refactoring existing code.

	Messages
:book:	:sparkles: Thanks for your contribution to Shields, @chris48s!

Generated by :no_entry_sign: dangerJS against 4f310689dfe214835dcd03e35df14ffb824e762e

[calebcartwright]

This is a bit of a "remove spacebar heating" change

I've not seen this one before and it gave me a good chuckle so thanks for sharing :smile:

I feel like the combination of the query param values is currently a silent error of sorts in that arch is accepted but completely ignored and which can result in a specious response relative to what the upstream system of record supports.

As such I feel like we should do something and between this proposed change or an alternative (e.g. return an error badge when the user provides a value for arch and sort=semver to explicitly indicate that's not supported) I think this is heading in the better direction.

Perhaps we could reduce the fallout by changing the defaulting behavior for arch and first check whether the user supplied anything, and using that conditionally to toggle between the old arch-agnostic behavior vs. new arch-specific filtering?

[chris48s]

Thanks for the suggestions. I had another look over the code and considered them.

I don't think we should make it an error if the arch param is used because there's not really a good reason why you can't use that param, other than we implemented it wrong.

My instinct was that your second suggestion might be better. If we were going to go down the road of saying that we look at all images of all architectures when the arch= param is not explicitly specified and only take it into account when it is explicitly passed, I think we should also do that for the other cases (where sort=date or tag is passed), which is also a behaviour change.

Even though the change in this PR makes a behaviour change, the change it makes (arch param works, and if you don't specify it the default is amd64) is the easiest behaviour to explain/document in the frontend. It is also consistent with how the image size badge works. At the moment, sort=semver does different things on the version and size badges.

[calebcartwright]

SGTM!

[chris48s]

Is that SGTM lets merge this, or SGTM lets do something else?