VMware: Rename results key to ansible_module_results

[wilmardo]

SUMMARY

Rename results key to ansible_module_results as shown when running the alpha version.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

vmware_guest_snapshot

ADDITIONAL INFORMATION

TASK [../win-cmdb-state-sync/ : Snapshot state = present] **********************************
 [WARNING]: Found internal 'results' key in module return, renamed to 'ansible_module_results'.

changed: [test]

TASK [../win-cmdb-state-sync/ : Snapshot state = present] ********************************
changed: [test -> localhost]

[ansibot]

cc @Akasurde @Tomorrow9 @ckotte @dav1x @lparkes @nerzhul @warthog9
click here for bot help

[ansibot]

@wilmardo, just so you are aware we have a dedicated Working Group for vmware.
You can find other people interested in this in #ansible-vmware on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

[Akasurde]

@wilmardo Thanks for opening this PR. Could you please add a changelog entry here and porting guide here.

needs_info

[wilmardo]

@Akasurde Added a changelog entry but is this really a noteworthy change? It is just an internal variable, it does nothing for the end-user (module return stays the same).

[sivel]

Technically this change from the beginning is a deviation for the user. Before 2.8 we didn't rename the key. The warning at least lets the user know they need to adjust their playbooks.

I wouldn't blindly rename it to match what Ansible does. I'd give it a better name, like snapshot_results.

[Akasurde]

@wilmardo Yes, it is noteworthy change. Since vmware_* modules are community supported and does not guarantee backward compatibility. So, user will refer porting guides and documentation for these kind of changes.

[Akasurde]

@pdellaert @dericcrago @Im0 @ckotte @jeking3 @Tomorrow9 @alongchamps @adarobin @jillr @goneri Could you please review this ?

[ansibot]

cc @acozine
click here for bot help

[wilmardo]

@sivel Good suggestion, updated!

it does nothing for the end-user (module return stays the same).

Excuses me for this, was confusing modules while testing this (saw nothing in the output, thought the key was for internal use only). I now 100% get why this is noteworthy change :)

Added an example to the documentation to clarify the change further.

[bcoca]

As @sivel states, i added the warning + rename to indicate 'module is doing something wrong' so using the same key as in the warning is discouraged. lgtm with latest changes.

[ansibot]

@wilmardo This PR contains @ mentions in at least one commit message. Those mentions can cause cascading notifications through GitHub and need to be removed. Please squash or amend your commits to remove the mentions.

click here for bot help

[wilmardo]

ready_for_review 🎉