-
Notifications
You must be signed in to change notification settings - Fork 20
add opportunity for metered/unmetered BillingMethod #26
Conversation
andreschab90
commented
Jun 20, 2017
- metered billingMethod will be used as a default value
- this PR should allow to use the unmetered BillingMethod
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thanks! I have a few minor change requests.
step.yml
Outdated
@@ -52,6 +52,12 @@ inputs: | |||
summary: "" | |||
description: "Project ARNs can be obtained using the AWS CLI 'devicefarm list-projects' command." | |||
is_required: true | |||
- billing_method: "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can change this to billingMethod: "METERED"
to set the default to metered as this is probably what most people will be using.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
step.yml
Outdated
opts: | ||
title: "Billing Method" | ||
summary: "" | ||
description: "use METERED or UNMETERED" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make this a little more descriptive? How about
"Specifies the billing method for a test run. Use `METERED` for free tier and pay-as-you-go billing, and `UNMETERED` to use pre-paid device slots. See [How to purchase device slots](https://docs.aws.amazon.com/devicefarm/latest/developerguide/how-to-purchase-device-slots.html) for more info."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks. updated
step.yml
Outdated
title: "Billing Method" | ||
summary: "" | ||
description: "use METERED or UNMETERED" | ||
is_required: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also please add this code to make this a drop-down list:
value_options:
- "METERED"
- "UNMETERED"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -283,6 +284,7 @@ fi | |||
echo_details "* device_farm_project: $device_farm_project" | |||
echo_details "* test_package_name: $test_package_name" | |||
echo_details "* test_type: $test_type" | |||
echo_details "* billing_method: $billing_method" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also around line 306 please add the following code to validate the expected option was passed:
options=("METERED" "UNMETERED")
validate_required_input_with_options "billing_method" "${billing_method}" "${options[@]}"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@fadookie PR is updated. Thanks war your amazing input :-) |
👍 great work |