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

man: explicitly distinguish "implicit dependencies" and "default dependencies" #6801

Merged
merged 2 commits into from Sep 14, 2017

Conversation

@johnlinp
Copy link
Contributor

@johnlinp johnlinp commented Sep 12, 2017

There are 2 commits here.

The first one moves a paragraph about recommending bus-based/socket-based activation, which I think is less relevant to automatic dependencies, from "Automatic Dependencies" section to "Description" section.

The second one is the fix that really fixes #6793.

Copy link
Member

@poettering poettering left a comment

looks pretty good, but two comments

Also the PR title is misleading now, it still says "automatic" instead of "implicit"

Loading

@@ -98,6 +98,12 @@
</refsect1>

<refsect1>
<title>Default Dependencies</title>

<para>There are no default dependencies for device units.</para>
Copy link
Member

@poettering poettering Sep 12, 2017

Choose a reason for hiding this comment

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

for the other pages you just added an xml comment. but for .device you added a section. Oversight or intended? (i think adding a comment instead is better)

Loading

Copy link
Contributor Author

@johnlinp johnlinp Sep 13, 2017

Choose a reason for hiding this comment

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

For unit type pages, I want to explicitly point out that there are no implicit/default dependencies because I mentioned them in systemd.unit(5).

For systemd.exec and systemd.resource-control, since they are only referenced in "Implicit Dependencies", I think it's okay to use comment.

So yes, it was intended. But I am open with changing them to comments.

Loading

Copy link
Contributor Author

@johnlinp johnlinp Sep 13, 2017

Choose a reason for hiding this comment

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

A few more words on my intention: when a user reads the sentence "For the implicit dependencies in each unit type, please refer to section "Implicit Dependencies" in respective man pages." in systemd.unit(5) and go see systemd.target(5) for the section "Implicit Dependencies", I want the user to know that target units don't have any implicit dependencies instead of being confused about "where is the Implicit Dependencies section?"

Loading

<title>Automatic Dependencies</title>
<title>Implicit Dependencies</title>

<para>The following dependencies are implicitly added.</para>
Copy link
Member

@poettering poettering Sep 12, 2017

Choose a reason for hiding this comment

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

maybe end this in a colon, and then use an itemizedlist for the paragaphs? (here and everywhere else)

Loading

Copy link
Contributor Author

@johnlinp johnlinp Sep 13, 2017

Choose a reason for hiding this comment

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

Okay, I will change it. Thanks.

Loading

@johnlinp johnlinp changed the title man: explicitly distinguish "automatic dependencies" and "default dependencies" man: explicitly distinguish "implicit dependencies" and "default dependencies" Sep 13, 2017
@johnlinp
Copy link
Contributor Author

@johnlinp johnlinp commented Sep 13, 2017

In the new commit, I modified the paragraphs into bullets style. The PR title and the commit message are updated too.

Please see my previous comment on why I didn't use comments in systemd.device and systemd.target, thank you.

Loading

@poettering poettering merged commit 21f0669 into systemd:master Sep 14, 2017
4 checks passed
Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

2 participants