Skip to content
This repository was archived by the owner on May 22, 2023. It is now read-only.

Code Exchange upgrade#31

Merged
po5i merged 6 commits into
nextfrom
code-exchange-upgrade
Jun 18, 2020
Merged

Code Exchange upgrade#31
po5i merged 6 commits into
nextfrom
code-exchange-upgrade

Conversation

@po5i

@po5i po5i commented Apr 22, 2020

Copy link
Copy Markdown
Contributor

Resolves #30

Carlos Villavicencio added 2 commits April 21, 2020 15:57
@po5i po5i force-pushed the code-exchange-upgrade branch from db43af1 to 157a079 Compare April 22, 2020 15:26
@po5i po5i changed the base branch from master to next May 11, 2020 15:29
@po5i po5i force-pushed the code-exchange-upgrade branch from 3d9ead2 to 3db1ce8 Compare May 20, 2020 20:29
Comment thread src/main/java/com/twilio/Webapp.java Outdated
Comment on lines +67 to +73
// wrap the phone number or client name in the appropriate TwiML verb
// by checking if the number given has only digits and format symbols
if(to.matches("^[\\d\\+\\-\\(\\) ]+$")) {
dialBuilder = dialBuilder.number(new Number.Builder(to).build());
} else {
dialBuilder = dialBuilder.client(new Client.Builder(to).build());
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe this can be in another function (private) just to make it clearer. Do you think it's worth it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nice suggestion. Want to take another look?

post("/voice", "application/x-www-form-urlencoded", (request, response) -> {
VoiceResponse voiceTwimlResponse;
String to = request.queryParams("To");
if (to != null) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great job refactoring this piece!

@fefi95 fefi95 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM!

@po5i po5i merged commit 5715c77 into next Jun 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Code Exchange quality checklist

2 participants