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: using standard OS tabs for registries docs #4497

Merged
merged 2 commits into from Oct 25, 2023

Conversation

themr0c
Copy link
Contributor

@themr0c themr0c commented Oct 24, 2023

What does this PR do?

  • Using standard "Windows, macOS, Linux" OS tabs to display options.
  • Slight refactoring: using snippets for repeated content.
  • Moved the content without rewriting it.
  • Minimal content fixes.

Screenshot/screencast of this PR

Screenshot_20231025_140902

What issues does this PR fix or reference?

Standardize navigation.

How to test this PR?

Signed-off-by: Fabrice Flore-Thébault <ffloreth@redhat.com>
@themr0c themr0c requested review from slemeur, benoitf and a team as code owners October 24, 2023 14:35
@themr0c themr0c requested review from dgolovin and feloy and removed request for a team October 24, 2023 14:35
@themr0c
Copy link
Contributor Author

themr0c commented Oct 24, 2023

This check is no good. Starting references to images with /docs... unfortunately does not work.

[vale] reported by reviewdog 🐶
[PodmanDesktop.Links] Consider starting the link with https:// or /, rather than ](img/adding-a-custom-registry.png)

@themr0c themr0c linked an issue Oct 25, 2023 that may be closed by this pull request
@@ -0,0 +1,37 @@
#### Prerequisites

- Podman.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume it needs to be running? Before the change that was required, and there is a restart in the steps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It only needs to be installed. You need to restart any podman process if it exists. I will clarify.

@benoitf
Copy link
Collaborator

benoitf commented Oct 25, 2023

I don't see why we add 2 new files (linux / windows) that are only included by one file.

why not keep the existing file (not extracting the content)

@themr0c
Copy link
Contributor Author

themr0c commented Oct 25, 2023

@benoitf for clarity and readability, I find it preferable to decouple the presentation layer (tabs) from the content (snippets). I plan to generalize this approach in every page containing tabs. I have not done that before only because previously, I was not familiar with the concept of "snippets" in Docusaurus.

…stry-linux.md

Signed-off-by: Fabrice Flore-Thébault <ffloreth@redhat.com>
Comment on lines +33 to +37
1. Restart Podman.

```shell-session
$ sudo systemctl restart podman
```
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "restart Podman" steps in various Linux procedures (here, proxy) are inconsistent and mostly irrelevant. It deserves its own issue.

@themr0c themr0c enabled auto-merge (squash) October 25, 2023 13:13
@themr0c themr0c merged commit c7ac765 into containers:main Oct 25, 2023
12 checks passed
@themr0c themr0c deleted the refactoring-registries branch October 25, 2023 13:40
@podman-desktop-bot podman-desktop-bot added this to the 1.6.0 milestone Oct 25, 2023
insecure = true
```

If you have multiple registries, you can add one `[[registry]]` block per registry:
Copy link
Contributor

Choose a reason for hiding this comment

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

: added by mistake

1. Open `registries.conf`.

```shell-session
$ sudo vi /etc/containers/registries.conf
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it in openshift docs / standards that sudo commands use # / comment out feature so it's not accidently copied-and-pasted? unsure if we should apply this here / other parts of PD documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

# command for a command by root. But usually people don't understand.

$ sudo command is correct, as you execute in the user context (sudo does the privilege escalation). Less prone to error because user forget to get root.


1. Click "Yes" to the insecure registry warning.

![Podman Desktop Registry Warning](img/registry-warning-insecure.png)
Copy link
Contributor

Choose a reason for hiding this comment

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

false positive on vale issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bad rule. I opened an issue. And am currently lost in regex: do not match strings starting by img/. The /docs... notation breaks the build...

$ sudo vi /etc/containers/registries.conf
```

1. Add the insecure registry: Add a new `[[registry]]` section for the URL of the insecure registry you want to use. For example, if your insecure registry is located at `http://registry.example.com`, add the following lines:
Copy link
Contributor

Choose a reason for hiding this comment

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

Accidental :?


If you have multiple registries, you can add one `[[registry]]` block per registry.

1. Save and exit the file.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need this step as it's implied by editing the file you save and exit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was already in the content. I changed as little content as possible. This might require a new pr with new scope.

@cdrage
Copy link
Contributor

cdrage commented Oct 25, 2023

I still have some comments on this PR @themr0c

@deboer-tim
Copy link
Collaborator

@benoitf for clarity and readability, I find it preferable to decouple the presentation layer (tabs) from the content (snippets). I plan to generalize this approach in every page containing tabs. I have not done that before only because previously, I was not familiar with the concept of "snippets" in Docusaurus.

Just to be upfront, I was also uncomfortable with the separation but let it go b/c I thought 'it's just one doc' and I like the user-side of the change. When I saw Florent's comments I realized I was wrong, but by then this was already merged.

I definitely do not think it is a good idea to use imports for every page with tabs - imports should be rare, and only for cases where one file is getting unmanageable or it actually makes sense for the same content to appear in two places. Why? First, it will make it harder for anyone else to contribute to docs, because it's harder to find things without searching, and having to spend extra time to understand the structure and how and where the content is going to appear, follow links, etc. Second, with 4x the files it would be harder to maintain, rename/move topics, slower to review PRs, etc for the same reasons.

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.

Use OS tabs for proxies documentation
5 participants