Skip to content

Update code example for pre-existing network - #14851

Merged
dockertopia merged 1 commit into
docker:masterfrom
grappler:patch-1
May 30, 2022
Merged

dockertopia merged 1 commit into
docker:masterfrom
grappler:patch-1

Conversation

@grappler

Copy link
Copy Markdown
Contributor

Proposed changes

"network default: network.external.name is deprecated in favor of network.name"

Update the code example to match the new syntax.

Related issues (optional)

Fixes #14683

@netlify

netlify Bot commented May 30, 2022

Copy link
Copy Markdown

✅ Deploy Preview for docsdocker ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 4f3bfc3
🔍 Latest deploy log https://app.netlify.com/sites/docsdocker/deploys/62947945320aa8000a69014a
😎 Deploy Preview https://deploy-preview-14851--docsdocker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@dockertopia dockertopia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glours Could you PTAL? The sample code looks fine to me based on #13958. TWISIT accepting changes in this PR would revert that update.

@grappler

Copy link
Copy Markdown
Contributor Author

@dockertopia Thank you for your review. I don't see any reasoning why #13958 was made. It appears to be incorrect for me. My suggested change was previously applied here: #6124

Please see the related comment: docker-archive/compose-cli#1856 (comment)

@thaJeztah

Copy link
Copy Markdown
Member

Hmm.. yes, #13958 looks a bit odd indeed; I think the name field was added in compose schema 2.3 and 3.5 (through this PR: docker/compose#5448)

@glours glours left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM regarding the Compose specification

@glours

glours commented May 30, 2022

Copy link
Copy Markdown
Contributor

@grappler Thanks for catching this one

@dockertopia dockertopia left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @grappler. That was a good catch. Felt compelled to double check based on previous change to the file.

@dockertopia
dockertopia merged commit a836a2a into docker:master May 30, 2022
@grappler
grappler deleted the patch-1 branch May 30, 2022 15:50
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.

Using 'external' marked as deprecated by Compose v2.5

4 participants