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

report: misc cleanup #25597

Merged
merged 5 commits into from
Jan 22, 2019
Merged

report: misc cleanup #25597

merged 5 commits into from
Jan 22, 2019

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Jan 20, 2019

It's probably easiest to review this as individual commits.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@cjihrig build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2308/pipeline

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jan 20, 2019
addaleax
addaleax reviewed Jan 21, 2019
View reviewed changes
src/node_report.cc Outdated Show resolved Hide resolved
@addaleax addaleax mentioned this pull request Jan 21, 2019
Closed
2 tasks
@addaleax
Copy link
Member

addaleax commented Jan 22, 2019 •
edited by cjihrig
Loading

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

EDIT(cjihrig): CI was green.

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 22, 2019
@cjihrig
report: remove unnecessary intermediate variable
ad2b42e 
PR-URL: nodejs#25597
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
@cjihrig
report: remove unnecessary includes
04c9f84 
PR-URL: nodejs#25597
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
@cjihrig
report: use uv_pid_t instead of custom PID_TYPE
5003314 
PR-URL: nodejs#25597
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
@cjihrig
report: simplify option checking
a022337 
Also update the code for house style.

PR-URL: nodejs#25597
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
@cjihrig
report: simplify rlimit to JSON logic
1ef175e 
PR-URL: nodejs#25597
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
@cjihrig
Copy link
Contributor Author

cjihrig commented Jan 22, 2019 •
edited
Loading

Landed in 38ab1e9...1ef175e.

@cjihrig cjihrig merged commit 1ef175e into nodejs:master Jan 22, 2019
@cjihrig cjihrig deleted the report branch January 22, 2019 23:06
@addaleax addaleax added the report Issues and PRs related to process.report. label Jan 22, 2019
@richardlau richardlau mentioned this pull request Jan 23, 2019
Merged
targos pushed a commit that referenced this pull request Jan 24, 2019
@cjihrig @targos
report: remove unnecessary intermediate variable
42bbe58 
PR-URL: #25597
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
targos pushed a commit that referenced this pull request Jan 24, 2019
@cjihrig @targos
report: remove unnecessary includes
213eddd 
PR-URL: #25597
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
targos pushed a commit that referenced this pull request Jan 24, 2019
@cjihrig @targos
report: use uv_pid_t instead of custom PID_TYPE
c598d98 
PR-URL: #25597
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
targos pushed a commit that referenced this pull request Jan 24, 2019
@cjihrig @targos
report: simplify option checking
a02b621 
Also update the code for house style.

PR-URL: #25597
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
targos pushed a commit that referenced this pull request Jan 24, 2019
@cjihrig @targos
report: simplify rlimit to JSON logic
0f91e03 
PR-URL: #25597
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2019
Merged
@snyk-bot snyk-bot mentioned this pull request Aug 11, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax addaleax approved these changes

@lundibundi lundibundi lundibundi approved these changes

@refack refack refack approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. report Issues and PRs related to process.report.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cjihrig @nodejs-github-bot @addaleax @refack @lundibundi