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

fix: docker-compose file decoding #184

Merged
merged 3 commits into from
Oct 22, 2020

Conversation

n1ru4l
Copy link
Collaborator

@n1ru4l n1ru4l commented Sep 30, 2020

Closes #183

@n1ru4l n1ru4l changed the title Docker compose decoding fix: docker-compose file decoding Sep 30, 2020
@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Sep 30, 2020

@erikengervall Let me know what you think :)

@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Oct 12, 2020

@erikengervall ping :)

@erikengervall
Copy link
Owner

@erikengervall Let me know what you think :)

Sorry for the late reply - been a busy few weeks 🙂

I'll take a look tonight! 🙌

@erikengervall
Copy link
Owner

Ran the unit tests as a sanity check and it produces these changes for me locally:

Screenshot 2020-10-12 at 17 48 58

I'm running docker desktop 2.4.0.0

Perhaps we should update the snapshots to the most recent version? WDYT?

@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Oct 12, 2020

@erikengervall yes! I actually thought that I did update them already 😅

@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Oct 19, 2020

It seems like the port format got reverted to in 1.27.4: docker/compose@1.27.3...1.27.4 (https://docs.docker.com/compose/release-notes/#1274)

I would stay we should steel keep the decoding, as it gives another layer of confidence. Please let me know what you think!

@n1ru4l
Copy link
Collaborator Author

n1ru4l commented Oct 21, 2020

@erikengervall Any ideas what's wrong with the unit tests? Some checks are not reported but required? I would love to see this merged quickly! Gonna pick up and finish #167 afterwards 🙂

@erikengervall
Copy link
Owner

@erikengervall Any ideas what's wrong with the unit tests? Some checks are not reported but required? I would love to see this merged quickly! Gonna pick up and finish #167 afterwards 🙂

I'll take a look when I get home later today :)

@erikengervall
Copy link
Owner

@erikengervall Any ideas what's wrong with the unit tests? Some checks are not reported but required? I would love to see this merged quickly! Gonna pick up and finish #167 afterwards 🙂

I'll take a look when I get home later today :)

Alright idk why GitHub Actions are acting up, the tests are seemingly passing and I've tried running them locally with the most recent changes.

I'll merge this and create a new release 🙂

@erikengervall erikengervall merged commit 23a5d87 into erikengervall:master Oct 22, 2020
@erikengervall
Copy link
Owner

@n1ru4l changes included in 2.1.0 🙂

@n1ru4l n1ru4l deleted the docker-compose-decoding branch October 22, 2020 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docker-compose port format has changed
2 participants