Skip to content

Conversation

@Thorben-D
Copy link

@Thorben-D Thorben-D commented Apr 5, 2024

This PR adds documentation for theforeman/foreman_ansible#707

Copy link
Member

@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.

Can you link to where this was implemented? That makes it easier to cross reference.

@Thorben-D
Copy link
Author

@ekohl
Whoops, I forgot about that. Added to the main post.

. Click *Import from YAML-File* in the toolbar.
+
A wizard opens to show you the next steps to import/override Ansible variables.
Ensure variables are unique per role.
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
Ensure variables are unique per role.
Ensure that variables are unique per Ansible role.

What happens if they're not?

Copy link
Author

Choose a reason for hiding this comment

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

In case a variable is present > 1 times in the UI, i.e.:

  • var0 <--
  • var1
  • var2
  • var0 <--

the user is not allowed to continue, as that would make the variable ambiguous.
If a variable is present that has been imported earlier, the user may proceed, but must understand that the variable's type and default value will be overridden.
Appropriate warnings are shown in both cases.

@pr-processor pr-processor bot added Waiting on contributor Requires an action from the author and removed Not yet reviewed labels Apr 8, 2024
@Thorben-D Thorben-D force-pushed the issue/OR-4615_ansible_variables_yaml_doc_upstream branch from 5c9b6b7 to 93d6143 Compare April 8, 2024 06:51
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Apr 8, 2024
Copy link
Author

@Thorben-D Thorben-D left a comment

Choose a reason for hiding this comment

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

@maximiliankolb
I addressed the issues and clarified "uniqueness".

Comment on lines 12 to 14
. Click *Import from YAML-File* in the toolbar.
+
A wizard opens to show you the next steps to import and override Ansible variables.
Copy link
Author

Choose a reason for hiding this comment

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

This line basically tells the user to just "follow the wizard". This was done in light of #2490.
Since my wizard also contains helpertexts and is laid out in a generally self-explanatory fashion, I found it fitting.

For the wording, I oriented myself towards:
https://github.com/theforeman/foreman-documentation/blob/master/guides/common/modules/con_standardizing-content-view-empty-states.adoc?plain=1#L6

. Click *Import from YAML-File* in the toolbar.
+
A wizard opens to show you the next steps to import/override Ansible variables.
Ensure variables are unique per role.
Copy link
Author

Choose a reason for hiding this comment

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

In case a variable is present > 1 times in the UI, i.e.:

  • var0 <--
  • var1
  • var2
  • var0 <--

the user is not allowed to continue, as that would make the variable ambiguous.
If a variable is present that has been imported earlier, the user may proceed, but must understand that the variable's type and default value will be overridden.
Appropriate warnings are shown in both cases.

@Thorben-D Thorben-D force-pushed the issue/OR-4615_ansible_variables_yaml_doc_upstream branch from 93d6143 to beaad55 Compare April 8, 2024 07:43
@maximiliankolb maximiliankolb deleted the issue/OR-4615_ansible_variables_yaml_doc_upstream branch April 25, 2025 09:48
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.

3 participants