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

test: var to const/let in test-tls-set-ciphers #9877

Closed
wants to merge 1 commit into from
Closed

test: var to const/let in test-tls-set-ciphers #9877

wants to merge 1 commit into from

Conversation

rajatkumar
Copy link
Contributor

@rajatkumar rajatkumar commented Dec 1, 2016 •
edited
Loading

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change

this change is part of code and learn (NINA-2016)

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Dec 1, 2016
@imyller
Copy link
Member

imyller commented Dec 1, 2016

@rajatkumar May I kindly ask you to format the commit message as described in CONTRIBUTING guidelines.

@imyller
Copy link
Member

imyller commented Dec 1, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/5045/

@mscdex mscdex added the tls Issues and PRs related to the tls subsystem. label Dec 1, 2016
@rajatkumar
test: var to const/let in test-tls-set-ciphers
b850b74 
this change is part of code and learn (NINA-2016)
@rajatkumar rajatkumar changed the title change all var to const/let test: var to const/let in test-tls-set-ciphers Dec 1, 2016
@imyller imyller added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Dec 1, 2016
@rajatkumar
Copy link
Contributor Author

@imyller I have ammended the commit message and also updated the PRs comment. Please let me know if this looks fine.

@imyller
Copy link
Member

imyller commented Dec 1, 2016

@rajatkumar Looks great! 👍

@rajatkumar
Copy link
Contributor Author

@imyller how do we re-run the ci tests?

targos
targos approved these changes Dec 3, 2016
View reviewed changes
Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

CI: https://ci.nodejs.org/job/node-test-pull-request/5143/

jasnell
jasnell approved these changes Dec 3, 2016
View reviewed changes
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for the PR and for participating in Code and Learn! Welcome to Node.js :-)

@jasnell
Copy link
Member

jasnell commented Dec 4, 2016

Failures appear to be unrelated

jasnell pushed a commit that referenced this pull request Dec 4, 2016
@rajatkumar @jasnell
test: var to const/let in test-tls-set-ciphers
639ddcb 
this change is part of code and learn (NINA-2016)

PR-URL: #9877
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Dec 4, 2016

Landed in 639ddcb. thank you!

@jasnell jasnell closed this Dec 4, 2016
addaleax pushed a commit that referenced this pull request Dec 5, 2016
@rajatkumar @addaleax
test: var to const/let in test-tls-set-ciphers
13cc6a0 
this change is part of code and learn (NINA-2016)

PR-URL: #9877
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Dec 5, 2016
Merged
2 tasks
addaleax pushed a commit to addaleax/node that referenced this pull request Dec 8, 2016
@rajatkumar @addaleax
test: var to const/let in test-tls-set-ciphers
c5bfe90 
this change is part of code and learn (NINA-2016)

PR-URL: nodejs#9877
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jmdarling pushed a commit to jmdarling/node that referenced this pull request Dec 8, 2016
@rajatkumar
test: var to const/let in test-tls-set-ciphers
068f92c 
this change is part of code and learn (NINA-2016)

PR-URL: nodejs#9877
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
@rajatkumar @MylesBorins
test: var to const/let in test-tls-set-ciphers
6c1c062 
this change is part of code and learn (NINA-2016)

PR-URL: #9877
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 21, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@targos targos targos approved these changes

@princejwesley princejwesley princejwesley approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. test Issues and PRs related to the tests. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.