Skip to content

docs: editorial improvements, typo fixes - #4712

Merged
thaJeztah merged 1 commit into
docker:masterfrom
dvdksn:docs-tier1-fresshness-q4
Dec 19, 2023
Merged

docs: editorial improvements, typo fixes#4712
thaJeztah merged 1 commit into
docker:masterfrom
dvdksn:docs-tier1-fresshness-q4

Conversation

@dvdksn

@dvdksndvdksn commented Dec 13, 2023

Copy link
Copy Markdown
Contributor

@dvdksn

Copy link
Copy Markdown
ContributorAuthor

@thaJeztah let me see if I can carry a couple follow-ups from the previous refresh as well

@dvdksn
dvdksn marked this pull request as draft December 13, 2023 15:07

@thaJeztahthaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

overall looking good; left some comments/suggestions, and recalled some "related" changes that were never finished.

Comment threaddocs/reference/commandline/attach.md Outdated
Comment threaddocs/reference/commandline/attach.md Outdated
Comment threaddocs/reference/commandline/attach.md Outdated
Comment on lines +194 to +195
> TLS version 1.0 and higher is supported. Protocols SSLv3 and below are not
> supported for security reasons.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remind me; we need to verify what TLS versions we support now (it's very likely we no longer support 1.0)

```

### Allow push of nondistributable artifacts
### Allow push of non-distributable artifacts

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remind me; we need to check the status on this one. I know the "foreign layers" feature was deprecated by the OCI, and we may be enabling this by default now (or are planning to)

Comment threaddocs/reference/commandline/dockerd.md Outdated
Comment threaddocs/reference/commandline/dockerd.md
Comment threaddocs/reference/commandline/dockerd.md Outdated
Comment on lines +178 to +183
- config (`config=<name or id>`)
- container (`container=<name or id>`)
- daemon (`daemon=<name or id>`)
- event (`event=<event action>`)
- image (`image=<repository or tag>`)
- label (`label=<key>` or `label=<key>=<value>`)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reminds me I had a PR to rewrite this to use tables, but some of the combinations in my examples were wrong, as combining some did not produce the same as --type 🤔 (IIRC); perhaps you're interested in carrying that PR though;

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Let me carry this one, but I'll do it in a follow-up as not to block this one since there's an open technical question

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need to merge the events and system_events pages. Currently information is spread between them (and they may be out of sync).

I recall doing that as part of my "WIP use canonical URLs" but that got bit-rot.

@dvdksn
dvdksnforce-pushed the docs-tier1-fresshness-q4 branch from ae456a8 to 08f3affCompareDecember 13, 2023 16:57
@dvdksn
dvdksn marked this pull request as ready for review December 13, 2023 19:00
@dvdksndvdksn changed the title docs: minor editorial improvements, typo fixesdocs: editorial improvements, typo fixesDec 13, 2023
@dvdksn

Copy link
Copy Markdown
ContributorAuthor

@thaJeztah I pushed an additional update as a fixup (to make it easier to review), see
2b252a8

@thaJeztahthaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

overall LGTM (thanks!)

found a couple of minor issues, and one or two that may need checking (sudo and env-var, and perhaps update that redirected URL)

`tar` UNIX format and can be compressed with any one of the 'xz', 'bzip2',
'gzip' or 'identity' (no compression) formats.
`tar` Unix format and can be compressed with any one of the `xz`, `bzip2`,
`gzip` or `identity` (no compression) formats.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think zstd may now also work, but we'd have to check if it does (so no need to change for this PR)


This will build an image for a compressed context read from `STDIN`. Supported
formats are: bzip2, gzip and xz.
formats are: `bzip2`, `gzip` and `xz`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here (also for follow-ups)

Comment threaddocs/reference/commandline/build.md Outdated
Comment threaddocs/reference/commandline/config_rm.md Outdated
Comment threaddocs/reference/commandline/context_create.md Outdated
Comment on lines +865 to +866
$ export DOCKER_TMPDIR=/mnt/disk2/tmp
$ /usr/local/bin/dockerd --data-root /var/lib/docker -H unix:// > /var/lib/docker-machine/docker.log 2>&1
$ sudo dockerd --data-root /var/lib/docker -H unix:// > /var/lib/docker-machine/docker.log 2>&1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh; does this work? sudo may be omitting the DOCKER_TMPDIR env-var (either requires sudo -E, or we should inline the env-var instead of exporting.

Also wondering if we should leave the > /var/lib/docker-machine/docker.log 2>&1 as an exercise to the reader (managing detached processes can be fiddly)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I don't even know what the redirect is doing in this case so I am happy to drop it

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@dvdksn
dvdksnforce-pushed the docs-tier1-fresshness-q4 branch from 2b252a8 to db7c7ecCompareDecember 19, 2023 16:14
@codecov-commenter

codecov-commenter commented Dec 19, 2023

Copy link
Copy Markdown

Codecov Report

Merging #4712 (2b252a8) into master (103840e) will decrease coverage by 0.03%.
Report is 21 commits behind head on master.
The diff coverage is n/a.

❗ Current head 2b252a8 differs from pull request most recent head 6fd4cff. Consider uploading reports for the commit 6fd4cff to get more accurate results

Additional details and impacted files
@@ Coverage Diff @@## master #4712 +/- ##
==========================================
- Coverage 59.68% 59.65% -0.03% 
==========================================
Files 287 285 -2 Lines 24865 24757 -108 ==========================================
- Hits 14841 14770 -71 + Misses 9138 9100 -38 - Partials 886 887 +1 

Signed-off-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>
@dvdksn
dvdksnforce-pushed the docs-tier1-fresshness-q4 branch from db7c7ec to 6fd4cffCompareDecember 19, 2023 16:15
@thaJeztahthaJeztah added this to the 25.0.0 milestone Dec 19, 2023

@thaJeztahthaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
thaJeztah merged commit 5ba998d into docker:masterDec 19, 2023
@dvdksn
dvdksn deleted the docs-tier1-fresshness-q4 branch December 19, 2023 18:42
@thaJeztahthaJeztah mentioned this pull request May 14, 2026
Sign up for freeto 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.

3 participants

@dvdksn@codecov-commenter@thaJeztah