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

Minor style updates related to UNIX #163

Conversation

bredamc
Copy link
Contributor

@bredamc bredamc commented Feb 22, 2022

First set of changes: UNIX

First set of changes: UNIX
@bredamc
Copy link
Contributor Author

bredamc commented Feb 22, 2022

This PR is a work in progress and is not yet ready for review.

Related issue: #147

@bergerhoffer bergerhoffer added the General update General updates to the guide or repo label Feb 22, 2022
@buildbricks buildbricks changed the title Multiple minor style updates [WIP] Multiple minor style updates Feb 23, 2022
@bredamc bredamc changed the title [WIP] Multiple minor style updates [WIP] Minor style updates related to UNIX Feb 23, 2022
@bredamc bredamc changed the title [WIP] Minor style updates related to UNIX [WIP] Minor UNIX style updates Feb 23, 2022
@bredamc bredamc changed the title [WIP] Minor UNIX style updates Minor style updates related to UNIX Feb 23, 2022
@bredamc
Copy link
Contributor Author

bredamc commented Mar 1, 2022

Hi @joaedwar! It is indeed ready for review, thank you! My apologies for not notifying potential reviewers!

@buildbricks
Copy link
Contributor

Thanks a million Breda! Will take a look

@@ -335,7 +335,7 @@ Write this name in full the first time that you use it in a document. Subsequent
[discrete]
[[runlevel]]
==== image:images/yes.png[yes] runlevel (noun)
*Description*: A "runlevel" is a preset operating state on a Unix-like operating system. A system can be booted in to (that is, started up in to) any of several runlevels, each of which is represented by a single-digit integer. Each runlevel designates a different system configuration and allows access to a different combination of processes (that is, instances of executing programs). There are differences in the runlevels according to the operating system. Seven runlevels are supported in the standard Linux kernel.
*Description*: A "runlevel" is a preset operating state on a UNIX system and similar operating systems. A system can be booted in to (that is, started up in to) any of several runlevels, each of which is represented by a single-digit integer. Each runlevel designates a different system configuration and allows access to a different combination of processes (that is, instances of executing programs). There are differences in the runlevels according to the operating system. Seven runlevels are supported in the standard Linux kernel.
Copy link
Contributor

@ndeevy ndeevy Mar 1, 2022

Choose a reason for hiding this comment

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

Fab improvements here @bredamc!

This comment is not part of your changes so feel free to ignore if it's beyond scope, but I wonder could we improve the second sentence, it seems unnecessarily clunky and troublesome for translation. Mainly because of the parentheses and the verb+preposition weirdness of ' started up in to'. Could probably avoid the word 'boot' altogether anyway. I don't think it'd change the meaning detrimentally to simplify it to something like:

You can start a system into several runlevels, each of which is represented by a single-digit integer.

could replace the second set while we're there too:

Each runlevel designates a different system configuration and allows access to a different combination of instances of executing programs.

Copy link
Contributor Author

@bredamc bredamc Mar 2, 2022

Choose a reason for hiding this comment

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

Thanks so much for your review, @ndeevy! I agree that the entire paragraph could use an edit, but I think it might be better to edit in a separate PR? I don't have enough expertise in this particular area so it would require significant review, which might delay the current PR. For example, is the system booted in several (or one of several) runlevels? Also, I think "processes" might be the better understood term, rather than "instances of executing programs". And is "allows" the correct verb here, or would "provides" be sufficient (not sure whether permissions are involved)? So many questions :)

Copy link
Contributor

Choose a reason for hiding this comment

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

@bredamc yeah, you're so right. It's potential can of worms territory. I can try fix it down the road so as not to delay this PR's changes being merged ;-]

@ndeevy
Copy link
Contributor

ndeevy commented Mar 1, 2022

These changes LGTM @bredamc
Just on minor comment on a preexisting bit of the content you can consider or ignore as you like :-]

Copy link
Collaborator

@bergerhoffer bergerhoffer left a comment

Choose a reason for hiding this comment

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

This LGTM, approving!

@Amrita42 @ndeevy Usually we'd just want to worry about reviewing what has been changed in the PR as opposed to existing text. Definitely consider submitting what you see in a separate PR though!

@Amrita42
Copy link

Amrita42 commented Mar 9, 2022

This LGTM, approving!

@Amrita42 @ndeevy Usually we'd just want to worry about reviewing what has been changed in the PR as opposed to existing text. Definitely consider submitting what you see in a separate PR though!

I have resolved one comment which this PR does not cover @bergerhoffer but two comments are for the modified content.

@Amrita42 Amrita42 merged commit 6f841c7 into redhat-documentation:master Mar 9, 2022
@bredamc bredamc deleted the breda_minor_style_updates_22feb2022 branch March 10, 2022 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General update General updates to the guide or repo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants