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

Re-review of bare metal chapter #42

Merged
merged 4 commits into from
Oct 8, 2019

Conversation

lzap
Copy link
Member

@lzap lzap commented Oct 4, 2019

A lot of changes, most of them are actually not related to upstreamize process.

. `tfm-rubygem-foreman_discovery`
endif::[]
. `foreman-discovery-image`
. `rubygem-smart_proxy_discovery`
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if . means a bullet, need to check.

Copy link
Member

Choose a reason for hiding this comment

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

It means number, ordered list, so change to * for foreman ifeval because you can't have a list of 1

Copy link
Member Author

Choose a reason for hiding this comment

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

OK

Copy link
Member

Choose a reason for hiding this comment

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

Note tfm- is a EL7 thing. Once we'll support EL8, it will change. For manual installs you can also use foreman-plugin-discovery which should be provided by the correct package. Similar we also have foreman-proxy-plugin-discovery instead of rubygem-smart_proxy_discovery (which will change to tfm-rubygem-smart_proxy_discovery in Foreman 1.24). Please do test this before putting in a manual though :)

ifeval::["{build}" == "satellite"]
[NOTE]
Discovery `kexec` is a Technology Preview feature.
endif::[]
Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed it is, it's a painful one and we are getting rid of it. We need to be very explicit to maybe stop users from using it before they get into trouble.

Copy link
Member

Choose a reason for hiding this comment

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

There are more docs on this that I am weeding out of another guide and trying to put in here. I keep getting distracted from it. Maybe we can look at it together on one of our calls.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's going away anyway in a year or something. I would not be worried too much.

Copy link
Member

@melcorr melcorr left a comment

Choose a reason for hiding this comment

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

Some changes requested.
I commited some grammar changes, you can squash the commit if you are OK with the changes.

guides/doc-Provisioning_Guide/topics/Bare_Metal.adoc Outdated Show resolved Hide resolved
guides/doc-Provisioning_Guide/topics/Bare_Metal.adoc Outdated Show resolved Hide resolved
guides/doc-Provisioning_Guide/topics/Bare_Metal.adoc Outdated Show resolved Hide resolved
guides/doc-Provisioning_Guide/topics/Bare_Metal.adoc Outdated Show resolved Hide resolved
guides/doc-Provisioning_Guide/topics/Bare_Metal.adoc Outdated Show resolved Hide resolved
guides/doc-Provisioning_Guide/topics/Bare_Metal.adoc Outdated Show resolved Hide resolved
guides/doc-Provisioning_Guide/topics/Bare_Metal.adoc Outdated Show resolved Hide resolved
guides/doc-Provisioning_Guide/topics/Bare_Metal.adoc Outdated Show resolved Hide resolved
ifeval::["{build}" == "satellite"]
[NOTE]
Discovery `kexec` is a Technology Preview feature.
endif::[]
Copy link
Member

Choose a reason for hiding this comment

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

There are more docs on this that I am weeding out of another guide and trying to put in here. I keep getting distracted from it. Maybe we can look at it together on one of our calls.

@melcorr
Copy link
Member

melcorr commented Oct 4, 2019

I found another thing:

Please search the file for this

repo --name=rhel --baseurl=http://download/released/RHEL-7/7.4/Server/x86_64/os/
repo --name=sat --baseurl=http://download2/nightly/Satellite/{ProductVersion}/candidate/latest-Satellite-{ProductVersion}-RHEL-7/compose/Satellite/x86_64/os/

What should we do with this link, or is it OK?

@lzap
Copy link
Member Author

lzap commented Oct 7, 2019

Commas and articles are the thing for me. I am wondering, if you spot some common mistakes can you start tracking them and then put them into the README. I can see that "Click OK to save" is a pattern I need to learn and a good candidate.

@lzap
Copy link
Member Author

lzap commented Oct 7, 2019

What should we do with this link, or is it OK?

I think it's ok. It's in the file anyway and it's generic "download" hostname which is probably safe.

@lzap
Copy link
Member Author

lzap commented Oct 7, 2019

Added new commit.

@melcorr melcorr merged commit 69bed37 into theforeman:master Oct 8, 2019
@lzap lzap deleted the bare-metal-rereview branch October 8, 2019 12:23
ianf77 pushed a commit to ianf77/foreman-documentation that referenced this pull request Feb 14, 2022
* Re-review of bare metal chapter

* grammar/style updates

* Incorporated some comments

* Removed unwanted changes
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.

None yet

3 participants