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

docs: clarify container label usage #1319

Merged
merged 7 commits into from
Jul 18, 2022
Merged

Conversation

EDIflyer
Copy link
Contributor

As discussed in #1318 - just to aid clarity for new users and also make docker-compose element clear.

As discussed in containrrr#1318 - just to aid clarity for new users and also make docker-compose element clear.
@piksel
Copy link
Member

piksel commented Jul 12, 2022

You can make the examples tabbed, like on the start page like this:

watchtower/docs/index.md

Lines 47 to 65 in 30f36b3

=== "docker run"
```bash
$ docker run -d \
--name watchtower \
-v /var/run/docker.sock:/var/run/docker.sock \
containrrr/watchtower
```
=== "docker-compose.yml"
```yaml
version: "3"
services:
watchtower:
image: containrrr/watchtower
volumes:
- /var/run/docker.sock:/var/run/docker.sock
```

@codecov
Copy link

codecov bot commented Jul 12, 2022

Codecov Report

Merging #1319 (d6f3749) into main (739f328) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1319   +/-   ##
=======================================
  Coverage   63.82%   63.82%           
=======================================
  Files          23       23           
  Lines        1534     1534           
=======================================
  Hits          979      979           
  Misses        465      465           
  Partials       90       90           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30f36b3...d6f3749. Read the comment docs.

@EDIflyer
Copy link
Contributor Author

EDIflyer commented Jul 12, 2022

@piksel good suggestion - still getting my head round mkdocs - have tweaked to 3 tabs for the 2 scenarios, hopefully have got it right but obviously doesn't show up on Github's markdown interpreter!

@EDIflyer EDIflyer closed this Jul 12, 2022
@EDIflyer EDIflyer deleted the patch-2 branch July 12, 2022 22:39
@EDIflyer EDIflyer restored the patch-2 branch July 12, 2022 22:40
@EDIflyer
Copy link
Contributor Author

sorry had renamed my branches so they made more sense, didn't realise that would automatically close the PR!

@EDIflyer EDIflyer reopened this Jul 12, 2022
docs/container-selection.md Outdated Show resolved Hide resolved
after stupidly using watchtower as the docker-compose example (doh!) have now updated to `someimage`, in-keeping with the other examples
Copy link
Member

@piksel piksel left a comment

Choose a reason for hiding this comment

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

Looks good overall, but I think the examples should be reduced a bit to make it clearer what is actually required.

docs/container-selection.md Outdated Show resolved Hide resolved
docs/container-selection.md Outdated Show resolved Hide resolved
docs/container-selection.md Outdated Show resolved Hide resolved
docs/container-selection.md Outdated Show resolved Hide resolved
EDIflyer and others added 4 commits July 18, 2022 10:32
Co-authored-by: nils måsén <nils@piksel.se>
Co-authored-by: nils måsén <nils@piksel.se>
Co-authored-by: nils måsén <nils@piksel.se>
Co-authored-by: nils måsén <nils@piksel.se>
@EDIflyer
Copy link
Contributor Author

Looks good overall, but I think the examples should be reduced a bit to make it clearer what is actually required.

Thanks - all very sensible suggestions 👍

@piksel piksel changed the title clarify documentation around setting label docs: clarify container label usage Jul 18, 2022
@piksel piksel merged commit 2aa01da into containrrr:main Jul 18, 2022
@EDIflyer EDIflyer deleted the patch-2 branch July 18, 2022 10:13
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.

2 participants