Skip to content

lib,report: improve signal name validation - #27137

Merged
cjihrig merged 2 commits into
nodejs:masterfrom
cjihrig:signal-validate
Apr 11, 2019
Merged

lib,report: improve signal name validation#27137
cjihrig merged 2 commits into
nodejs:masterfrom
cjihrig:signal-validate

Conversation

@cjihrig

@cjihrigcjihrig commented Apr 8, 2019

Copy link
Copy Markdown
Contributor
  • Add a signal name validator to enforce type and check for known signals.
  • Improve error message when the signal name casing is incorrect.
  • Use the new validator in the report module.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot

nodejs-github-bot commented Apr 8, 2019

Copy link
Copy Markdown
Collaborator

Lite-CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3200

@nodejs-github-botnodejs-github-bot added the process Issues and PRs related to the process subsystem. label Apr 8, 2019
@nodejs-github-bot

nodejs-github-bot commented Apr 9, 2019

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Apr 9, 2019•
edited by cjihrig
Loading

Copy link
Copy Markdown
Collaborator

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

EDIT(cjihrig): 💚

@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 9, 2019
@cjihrig
lib: add signal name validator
5b9e570
PR-URL: nodejs#27137
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@cjihrig
report: improve signal name validation
44a3acb
PR-URL: nodejs#27137
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@cjihrig

cjihrig commented Apr 11, 2019

Copy link
Copy Markdown
ContributorAuthor

Landed in 72f4a83...44a3acb. Thanks for the reviews.

@cjihrigcjihrig closed this Apr 11, 2019
@cjihrig
cjihrig deleted the signal-validate branch April 11, 2019 00:37
@cjihrig
cjihrig merged commit 44a3acb into nodejs:masterApr 11, 2019
This was referenced Apr 23, 2019
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnoordhuisbnoordhuisbnoordhuis approved these changes
@richardlaurichardlaurichardlau approved these changes
@BridgeARBridgeARBridgeAR approved these changes
+1 more reviewer
@Fishrock123Fishrock123Fishrock123 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.processIssues and PRs related to the process subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@cjihrig@nodejs-github-bot@bnoordhuis@Fishrock123@richardlau@BridgeAR