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

DT-951 DT-1012 SockJS Client - Add "roundTrips" and "timeout" parameters #2

Merged
merged 4 commits into from
Mar 6, 2015
Merged

DT-951 DT-1012 SockJS Client - Add "roundTrips" and "timeout" parameters #2

merged 4 commits into from
Mar 6, 2015

Conversation

thieupham-wf
Copy link
Contributor

@rmconsole-wf
Copy link

General Information

Ticket(s): DT-951 DT-1012
Code Review: #2
CI:
Automation Run:

RM Checklist

  • QA Review or Automation
  • Last Commit Reviewed
  • Smithy build successful (no build info found)
  • Smithy run against most recent commit (no build info found)
  • Smithy run in past 24 hours (no build info found)
  • Unit Tests Updated
    • Tests updated for Dart code changes
  • Security Review Completed if Applicable: (N/A)

Additional Information

    When this pull is merged I will use the following information:
    Version: sockjs-dart-client 1.0.0
    Release Ticket: DT-951

Last updated on Friday, March 06 04:58 PM CST

travissanderson-wf
travissanderson-wf reviewed Mar 3, 2015
View reviewed changes
lib/src/client.dart
: PROTOCOLS[protocol].roundTrips;

var to = this.timeout;
if (to == null || to < 1) {
Copy link

Choose a reason for hiding this comment

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

should this be == 0 instead of < 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

< 1 because timeout shouldn't be a negative number. if a negative number was passed in, it would be ignored and calculation would be done.

@travissanderson-wf
Copy link

+1

@rmconsole3-wf rmconsole3-wf changed the title DT-1012 SockJS Client - Add "roundTrips" and "timeout" parameters DT-951 DT-1012 SockJS Client - Add "roundTrips" and "timeout" parameters Mar 3, 2015
@johnryan-wf
Copy link

+1

1 similar comment
@bryonmarks-wf
Copy link
Contributor

+1

@bryonmarks-wf
Copy link
Contributor

+10

@katiesanderson-wf
Copy link
Contributor

QA +1

katiesanderson-wf added a commit that referenced this pull request Mar 6, 2015
@katiesanderson-wf
Merge pull request #2 from thieupham-wf/add_sockjs_client_parameters
6b281d4 
DT-951 DT-1012 SockJS Client - Add "roundTrips" and "timeout" parameters
@katiesanderson-wf katiesanderson-wf merged commit 6b281d4 into Workiva:master Mar 6, 2015
@travisreed-wf
Copy link

rosie

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@thieupham-wf @rmconsole-wf @travissanderson-wf @johnryan-wf @bryonmarks-wf @katiesanderson-wf @travisreed-wf