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

chore: Remove base-60 port quote warning from example compose.yaml #3982

Merged
merged 2 commits into from Apr 21, 2024

Conversation

polarathene
Copy link
Member

@polarathene polarathene commented Apr 19, 2024

This should not be relevant to users of docker compose which is the primary demographic.

However, I know that we have users with other compose clients that parse compose.yaml where it may still be a gotcha, so feel free to reject this PR 👍


Additional information

docker compose (v2) uses the v3 version of go-yaml for parsing YAML where this gotcha will not occur. This package intentionally avoids base-60 syntax that we have raised awareness in our example compose.yaml. The Go rewrite of docker compose did not exist back when we added this AFAIK, and the earlier Python based version of Docker Compose was more prevalent (now EOL), which is where the bug was probably being encountered.

It's worth noting that while base-60 syntax (aka sexagesimal) is valid for YAML 1.1, it's been removed with YAML 1.2 of the spec. YAML 1.2 spec was published in 2009Q3, so there's been adequate time for parsers in modern maintained software to update to that.

The last bug report to motivate the PR to add this warning comment was in March 2021, but the reporter did not provide much details about their environment. Presumably it'd be the fault of the YAML parser distributed with the docker-compose (Python) package though 🤷‍♂️ (plus maintainers had confirmed it on their own systems). So 12 years roughly may not have been adequate time 😅

The referenced compose spec still raises this awareness, which is fair as it's more generic towards implementers parsing YAML. It was added to the spec directly (Jan 2020) with many other changes and no PR process, no context provided.

This should not be relevant to users of `docker compose` which is the primary demographic.
@georglauterbach
Copy link
Member

That's a tough one. I always appreciate getting rid of old stuff that nobody uses anymore. In this case, I think it is warranted, too. I am just raising the concern that, in case we do see this issue popping back up, we should revert.

@polarathene polarathene enabled auto-merge (squash) April 21, 2024 23:27
@polarathene polarathene merged commit d739fe3 into master Apr 21, 2024
1 check passed
@polarathene polarathene deleted the chore/remove-base60-warning-compose-example branch April 21, 2024 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants