The Wayback Machine - https://web.archive.org/web/20230131200320/https://github.com/nodejs/node/pull/46428
Skip to content
Sign up
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.

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

process: print versions by sort #46428

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

process: print versions by sort #46428

wants to merge 10 commits into from
+28 −8

Conversation

Himself65
Copy link
Member

@Himself65 Himself65 commented Jan 30, 2023 •
edited

Fixes: #45630

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Jan 30, 2023
addaleax
addaleax reviewed Jan 30, 2023
View changes
src/node_process_object.cc Outdated Show resolved Hide resolved
@Himself65
Copy link
Member Author

I'm trying to use std::pair<name, version_number> then sort and create a new v8::Object

@Himself65 Himself65 changed the title src: sort version keys process: print versions by sort Jan 30, 2023
@Himself65 Himself65 marked this pull request as ready for review Jan 30, 2023
@Himself65 Himself65 added the process Issues and PRs related to the process subsystem. label Jan 30, 2023
@Himself65 Himself65 mentioned this pull request Jan 30, 2023
Open
@nodejs-github-bot
Copy link
Contributor

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

@Himself65 Himself65 requested a review from addaleax Jan 30, 2023
@nodejs-github-bot
Copy link
Contributor

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

addaleax
addaleax approved these changes Jan 30, 2023
View changes
src/node_process_object.cc Outdated Show resolved Hide resolved
src/node_process_object.cc Outdated Show resolved Hide resolved
Himself65 added 2 commits Jan 31, 2023
@Himself65
fixup!: fix code
b58b306
@Himself65
fixup!: lint code
119a91a 
Signed-off-by: himself65 <himself65@toeverything.info>
jasnell
jasnell reviewed Jan 31, 2023
View changes
src/node_process_object.cc Outdated Show resolved Hide resolved
@Himself65 @jasnell
fixup!: Update src/node_process_object.cc
62c66ca 
Co-authored-by: James M Snell <jasnell@gmail.com>
@Himself65
Copy link
Member Author

I'm confused that why build CI all crashed today

@jasnell
Copy link
Member

jasnell commented Jan 31, 2023

There was a GitHub issue with tarball sha's today that caused stuff all over the place to break. The rolled it back this afternoon. Pretty sure that's what hit us today.

@RaisinTen
Copy link
Member

Could you also add a test?

@Himself65
Copy link
Member Author

Could you also add a test?

I will do it later

addaleax
addaleax reviewed Jan 31, 2023
View changes
src/node_process_object.cc Outdated Show resolved Hide resolved
Himself65 and others added 2 commits Jan 31, 2023
@Himself65 @addaleax
Update src/node_process_object.cc
56aef73 
Co-authored-by: Anna Henningsen <github@addaleax.net>
@Himself65
fixup!: lint code
4da1528 
Signed-off-by: himself65 <himself65@toeverything.info>
addaleax
addaleax reviewed Jan 31, 2023
View changes
src/node_process_object.cc Outdated Show resolved Hide resolved
Himself65 and others added 2 commits Jan 31, 2023
@Himself65 @addaleax
Update src/node_process_object.cc
3c16f28 
Co-authored-by: Anna Henningsen <github@addaleax.net>
@Himself65
fixup!: improve test
9a34041 
Signed-off-by: himself65 <himself65@toeverything.info>
@Himself65
Copy link
Member Author

ive updated the test case

Himself65 and others added 2 commits Jan 31, 2023
@Himself65
Update test-process-versions.js
f4d7b16
@Himself65
fixup!: lint fix
a41a360 
Signed-off-by: himself65 <himself65@toeverything.info>
@Himself65 Himself65 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 31, 2023
@Himself65 Himself65 requested a review from RaisinTen Jan 31, 2023
@nodejs-github-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax

@jasnell jasnell

@RaisinTen RaisinTen

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++. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The version list should be sorted by name
5 participants
@Himself65 @nodejs-github-bot @jasnell @RaisinTen @addaleax
Morty Proxy This is a proxified and sanitized view of the page, visit original site.