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.

Sign up for GitHub

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

Jump to bottom

Fix var naming #55796

Merged
merged 2 commits into from May 8, 2019
Merged

Fix var naming #55796

merged 2 commits into from May 8, 2019

Conversation

stroebitzer
Copy link
Contributor

+label: docsite_pr

SUMMARY
ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
  • Feature Pull Request
  • New Module Pull Request
COMPONENT NAME
ADDITIONAL INFORMATION

@stroebitzer
Fix var naming
09822ab 
<!--- Your description here -->

+label: docsite_pr
@ansibot
Copy link
Contributor

ansibot commented Apr 26, 2019

cc @acozine
click here for bot help

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. docs This issue/PR relates to or includes documentation. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. small_patch support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Apr 26, 2019
jamescassell
jamescassell suggested changes Apr 29, 2019
View reviewed changes
Copy link
Contributor

@jamescassell jamescassell left a comment

Choose a reason for hiding this comment

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

The original version may have been incorrect, but the new version is certainly incorrect. Perhaps you meant address.address?

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. new_contributor This PR is the first contribution by a new community member. labels Apr 29, 2019
@acozine
Copy link
Contributor

acozine commented Apr 29, 2019

@stroebitzer thanks for taking the time to open a PR on the Ansible documentation!

I'm not sure what problem you're trying to solve in this PR, though. Can you post more information? What problem did you see with the existing example? Can you post before-and-after playbook output?

If you're interested in documentation, feel free to join our chat group on the #ansible-docs channel on freenode IRC.

@acozine acozine removed the needs_triage Needs a first human triage before being processed. label Apr 29, 2019
@stroebitzer
Copy link
Contributor Author

The playbook simply did not work in my environment. The variable "address" got regitstered in a previous step and used afterwards via "instance.address".

After changing the usage to "address" everything worked for me.

@acozine
Copy link
Contributor

acozine commented May 1, 2019 •
edited

I see the earlier task that registers address as a variable. I'd like to see this change tested before we merge (or close) it, and I do not have a GCE account to test it on. @rambleraptor or @erjohnso could you test out this revision?

@rambleraptor
Copy link
Contributor

Hello! Yeah, I'll test this out and get an answer for you today.

@rambleraptor
Copy link
Contributor

Yeah, this doesn't work for the same reasons that @acozine described. instance.address as placed in the guide isn't correct either. It should be address.address as @jamescassell said.

@acozine
Copy link
Contributor

acozine commented May 6, 2019

@rambleraptor thanks for the review - can you take a second look with the additional commit?

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels May 6, 2019
@rambleraptor
Copy link
Contributor

Looks good!

@acozine
Copy link
Contributor

acozine commented May 8, 2019

Thanks everybody!

@acozine acozine merged commit dae5564 into ansible:devel May 8, 2019
acozine pushed a commit to acozine/ansible that referenced this pull request May 10, 2019
@stroebitzer @acozine
Fix var naming (ansible#55796)
d378cba 
* Fix var naming in GCE guide

(cherry picked from commit dae5564)
@acozine acozine mentioned this pull request May 10, 2019
Merged
@ansible ansible locked and limited conversation to collaborators Aug 5, 2019
@dagwieers dagwieers added the docsite_pr This PR is created from documentation using the "Edit on GitHub" link. label Sep 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jamescassell jamescassell jamescassell approved these changes

Assignees
No one assigned
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. docs This issue/PR relates to or includes documentation. docsite_pr This PR is created from documentation using the "Edit on GitHub" link. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@stroebitzer @ansibot @acozine @rambleraptor @jamescassell @dagwieers