Skip to content

Conversation

@marcelmamula
Copy link

Disclaimer

This is breaking change as all variables were redesigned without backwards compatibility.
Previously used variables were not adhering to best practices and failed linting.

This activity is part of major redesign of all roles in preparation for 1.0 release. More details in #45

Changes

This role was completely broken down and rebuilt from ground up to follow Ansible best practices, Ansible 2.20 linting and Project guidelines.

  • Majority of variables removed and replaced with much more powerful definition variable
  • Assert for all definitions and variables added
  • Validation of all definitions against target host
  • Replicated whole functionality of community.sap_libs.sap_system_facts into native Ansible code to remove dependency.
    • It can still be used with new variable.
  • Code was refactored to adhere to best practices and all changes in ansible-core 2.19 and 2.20

Tests

This PR was tested on SLES for SAP Applications 15 SP6 and 16 targeting hosts with latest SAP BW4HANA installed.

FYI
@crweller
@nbttmbrg
@sean-freeman

@berndfinger berndfinger self-requested a review December 15, 2025 13:39
Copy link
Member

@sean-freeman sean-freeman left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@berndfinger berndfinger left a comment

Choose a reason for hiding this comment

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

I successfully tested most of the actions on a SAP S/4HANA 2023 system with HANA rev 88 on a RHEL 10 system. The requested changes are only about missing "The " at the beginning of sentences which start with "Following" (English style).

@sean-freeman
Copy link
Member

Really fussy final comment, if __command_user has private internal var prefix, shouldn't command_item? I am really nit-picking though....

@marcelmamula
Copy link
Author

marcelmamula commented Jan 12, 2026

Really fussy final comment, if __command_user has private internal var prefix, shouldn't command_item? I am really nit-picking though....

@sean-freeman
Loop item was always item until recently when it is no longer allowed so we switched to using unique loop_var, but it did not have internal designation __var, neither it is flagged for lacking role name because it is not fact, just var.

I think it would make sense, but it would have to come as project guideline as we do not follow it anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants