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

Adding comments for container options in qm.container #678

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ilimarx
Copy link

@ilimarx ilimarx commented Dec 19, 2024

No description provided.

@@ -35,10 +35,24 @@ LimitNOFILE=65536
TasksMax=50%

[Container]
# AddCapability
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would recommend here to use man quadlet also for details

Copy link
Author

Choose a reason for hiding this comment

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

Tried to copy some details from man quadlet, hope it's better now

Copy link
Collaborator

@Yarboa Yarboa Jan 14, 2025

Choose a reason for hiding this comment

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

Consider adding this link to upstream doc
https://docs.podman.io/en/v4.4/markdown/podman-systemd.unit.5.html#addcapability
for each Capability

Or that one, it is more accurate
man quadlet | awk '/AddCapability=$/,/^$/'

MHO it more accurate then copy paste content of changing description.
It could also be automated to check if parameters were changed,
I mean file parameters vs existence in manual

Copy link
Author

Choose a reason for hiding this comment

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

Added a link to the comment for AddCapability option. Not sure about the "upstream doc", I was told to write comments for options.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If some on told you, this is the place to comment it

# AddCapability
# -------------
# Grants all capabilities to the container, increasing flexibility but significantly
# reducing security.
AddCapability=all
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, last comments related to that,

Consider thinking of man command , repo is upstream code, that could run against different podman versions, this way, in case of qm failures due to quadlet file, user can check the installed qudlet manual and refer the supported capabilities.

qm.container Outdated Show resolved Hide resolved
qm.container Outdated Show resolved Hide resolved
qm.container Show resolved Hide resolved
@Yarboa
Copy link
Collaborator

Yarboa commented Jan 23, 2025

@ilimarx please squash all commits. this change should be one commit
There are 6 commits for one change.
do not forget to verify b4 force push that in your git log your author is filled
and run the precommit checks commands before pushing it again

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