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

Fix reviews issues for scaleway_lb #52234

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
5 participants
@sieben
Copy link
Contributor

sieben commented Feb 14, 2019

SUMMARY

Fix reviews from @pilou- for scaleway_lb

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • scaleway_lb
ADDITIONAL INFORMATION
$ ansible --version
ansible 2.8.0.dev0
  config file = None
  configured module search path = [u'/Users/sieben/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/sieben/workspace/ansible/lib/ansible
  executable location = /Users/sieben/workspace/ansible/bin/ansible
  python version = 2.7.15 (default, Feb 12 2019, 11:00:12) [GCC 4.2.1 Compatible Apple LLVM 10.0.0 (clang-1000.11.45.5)]
@dagwieers
Copy link
Member

dagwieers left a comment

I am inclined to reject this PR, but I like to understand why this change is proposed.

@@ -70,15 +70,15 @@
type: bool
default: 'no'
wait_timeout:
until:

This comment has been minimized.

@dagwieers

dagwieers Feb 21, 2019

Member

I don't think this new parameter name is acceptable.
It is confusing because we have a task directive named until.
But also it doesn't accept the same content or has the same purpose.

The original parameter name is much better, and I would keep it as-is.

required: false
default: 300
wait_sleep_time:
delay:

This comment has been minimized.

@dagwieers

dagwieers Feb 21, 2019

Member

Same remark here. Keep it as it is.

@sieben

This comment has been minimized.

Copy link
Contributor Author

sieben commented Feb 21, 2019

@dagwieers I've mostly done it for @pilou-
@pilou- Could you give your motivations to @dagwieers ?

@gundalow gundalow self-assigned this Feb 22, 2019

@ansibot ansibot added the stale_ci label Mar 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.