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

Update ansible docs according to docs.theforeman.org/nightly urls #45

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

girijaasoni
Copy link
Contributor

@girijaasoni girijaasoni commented Apr 12, 2024

Note: The satellite URLs will work for 6.15 or higher :)

Blocked by theforeman/foreman_ansible#711

Comment on lines 8 to 9
'ImportingRoles' => "#{ForemanThemeSatellite.documentation_root}/managing_configurations_using_ansible_integration_in_red_hat_satellite/getting_started_with_ansible_in_satellite_ansible#Importing_Ansible_Roles_and_Variables_ansible",
'Variables' => "#{ForemanThemeSatellite.documentation_root}/managing_configurations_using_ansible_integration_in_red_hat_satellite/getting_started_with_ansible_in_satellite_ansible#Overriding_Ansible_Variables_in_satellite_ansible",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if it's a good idea to replace these keys. It's possible that they are coded elsewhere as identifiers and if we remove them, then we lose the connection. But I'll try to find someone who really understands it to ack this.

Copy link
Contributor

Choose a reason for hiding this comment

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

It was correct to remove them, because they were removed from the source code.

@girijaasoni please remove the lines again.

Copy link
Contributor

@Lennonka Lennonka left a comment

Choose a reason for hiding this comment

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

We shortened the guide URL in 6.15, but the long variant is valid for 6.14. Ie. if cherry-picked, it won't work.

lib/foreman_theme_satellite/documentation.rb Outdated Show resolved Hide resolved
lib/foreman_theme_satellite/documentation.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

When you're taking the suggestions in theforeman/foreman_ansible#711 I think you'll need to override docs_url in the LinksController. We haven't don't that yet, but it needs to happen.

@ShimShtein
Copy link
Contributor

@ekohl

When you're taking the suggestions in theforeman/foreman_ansible#711 I think you'll need to override docs_url in the LinksController. We haven't don't that yet, but it needs to happen.

We already have override for the docs_url:

Although, once the PR gets in, we will need to add Managing_Configurations_Ansible and the chapter names to DOCS_GUIDES_LINKS dictionary.

@ekohl
Copy link
Contributor

ekohl commented Jun 27, 2024

I know why I couldn't find it: I had the master branch checked out, not develop. My bad

@girijaasoni girijaasoni force-pushed the ansible-docs-update branch from b7dd463 to 6e0933d Compare July 26, 2024 10:51
Copy link
Contributor

@Lennonka Lennonka left a comment

Choose a reason for hiding this comment

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

The Ansible changes look alright. I don't have an instance to test them, though.

@girijaasoni girijaasoni force-pushed the ansible-docs-update branch from 6e0933d to ad97f02 Compare July 26, 2024 12:20
lib/foreman_theme_satellite/documentation.rb Outdated Show resolved Hide resolved
lib/foreman_theme_satellite/documentation.rb Outdated Show resolved Hide resolved
@girijaasoni girijaasoni force-pushed the ansible-docs-update branch 2 times, most recently from f886c72 to fbfdedc Compare July 26, 2024 12:56
Comment on lines 87 to 88
'Managing_Configurations_Ansible' =>
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'Managing_Configurations_Ansible' =>
{
'Managing_Configurations_Ansible' => {

@girijaasoni girijaasoni force-pushed the ansible-docs-update branch 2 times, most recently from 214c18a to ba40b0e Compare July 31, 2024 10:58
@girijaasoni girijaasoni force-pushed the ansible-docs-update branch from ba40b0e to 5eac842 Compare July 31, 2024 10:59
@ShimShtein
Copy link
Contributor

@ekohl LGTM, any last comments?

@ekohl ekohl merged commit 13453ca into RedHatSatellite:develop Jul 31, 2024
24 of 29 checks passed
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.

4 participants