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: askx new -> change the representation layer for deployers #106

Merged
merged 5 commits into from
Apr 7, 2020

Conversation

Chih-Ying
Copy link
Contributor

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Contributor

@pbheemag pbheemag left a comment

Choose a reason for hiding this comment

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

Needs changes to new command's documentation and Alexa-Hosted-Skill-Commands.md file

@RonWang
Copy link
Contributor

RonWang commented Apr 7, 2020

Need a rebase so that the CI fix can be included.

Comment on lines 119 to 124
// eslint-disable-next-line array-callback-return
R.values(deployType).map((deployer) => {
if (answer.deployDelegate === deployer.OPTION_NAME) {
return callback(null, deployer.NAME);
}
});
Copy link
Contributor

@RonWang RonWang Apr 7, 2020

Choose a reason for hiding this comment

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

I think if not understanding that the loop will only hit the target once, putting the callback inside a for loop looks not good. The eslint actually makes sense and we should not ignore. I think you can use try

callback(null, R.find(R.propEq('OPTION_NAME', 'AWS Lambda'))(R.values(deployType)).NAME)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This solution is much better, it returns the first element of the list which matches the predicate, or undefined if no element matches.

Copy link
Contributor

@RonWang RonWang left a comment

Choose a reason for hiding this comment

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

Ship it!

               ~;
              ,/|\,
            ,/' |\ \,
          ,/'   | |  \
        ,/'     | |   |
      ,/'       |/    |
    ,/__________|-----' ,
   ___.....-----''-----/

@RonWang RonWang merged commit 0c7750c into develop Apr 7, 2020
@RonWang RonWang deleted the deployer-representation-fix branch April 7, 2020 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants