-
Notifications
You must be signed in to change notification settings - Fork 86
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
arrange docs after some cli parameters are deleted #337
Conversation
Codecov ReportBase: 28.30% // Head: 28.30% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #337 +/- ##
=======================================
Coverage 28.30% 28.30%
=======================================
Files 37 37
Lines 3674 3674
=======================================
Hits 1040 1040
Misses 2514 2514
Partials 120 120 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
README.md
Outdated
|
||
A CLI tool [ctr-remote](https://github.com/dragonflyoss/image-service/tree/master/contrib/ctr-remote) is alongside. Use Nydus `ctr-remote` to pull Nydus image or start container based on nydus image. | ||
Start container using `nydus-snapshotter` using `nerdctl` which has native nydus support. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using nerdctl
which has native nydus support to start a container via nydus-snapshotter
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
b45536a
to
379ef3e
Compare
README.md
Outdated
@@ -80,22 +79,16 @@ systemctl restart containerd | |||
Or you can start nydus-snapshotter manually. | |||
|
|||
```bash | |||
# `nydusd-path` is the path to nydusd binary. If `nydusd` and `nydus-image` are installed, `--nydusd-path` and `--nydusimage-path`can be omitted. | |||
# `nydusd-path` is the path to nydusd binary. If `nydusd` and `nydus-image` are installed and can be searched from `PATH`, `--nydusd-path` and `--nydusimage-path`can be omitted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why still --nydusd-path
, --nydusimage-path
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another PR https://github.com/containerd/nydus-snapshotter/pull/339/files is addressing this, which is clean up cli help message
README.md
Outdated
--nydusimg-path /usr/local/bin/nydus-image \ | ||
--log-to-stdout | ||
$ sudo ./containerd-nydus-grpc --config /etc/nydus/config.toml --nydusd-config /etc/nydus/nydusd-config.json \ | ||
--address /run/containerd-nydus/containerd-nydus-grpc.sock --log-to-stdout \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It the --address
can be ignored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Good idea. We can simplify the default command line further
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the flag --address
is removed
README.md
Outdated
|
||
A CLI tool [ctr-remote](https://github.com/dragonflyoss/image-service/tree/master/contrib/ctr-remote) is alongside. Use Nydus `ctr-remote` to pull Nydus image or start container based on nydus image. | ||
Start container using `nerdctl` which has native nydus support with `nydus-snapshotter`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe note >= 0.22
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Good work, please also help to update other docs (in the image-service repo). |
Sure. Will raise an according PR |
379ef3e
to
beffdc9
Compare
@changweige Once resolve the conflict, we can merge it. |
Signed-off-by: Changwei Ge <gechangwei@bytedance.com>
beffdc9
to
a802abd
Compare
It was resolved |
Because some CLI parameters are removed, we have to arrange the docs