Skip to content
This repository was archived by the owner on Mar 22, 2023. It is now read-only.

Conversation

@freenice12
Copy link
Contributor

No description provided.

@freenice12
Copy link
Contributor Author

오잉?! 둘 다 빌드가 깨졌습니다...

@outsideris
Copy link
Contributor

전에도 PR에 빌드가 돌았었나요? 헷갈리네요.

### 주요 변경사항

* **async_hooks**:
- 불안전을 야기하는 {Before,After}가 폐기 예정입니다. (Ali Ijaz Sheikh) [#18513](https://github.com/nodejs/node/pull/18513)
Copy link
Contributor

Choose a reason for hiding this comment

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

PR을 보면 emitBeforeemitAfter에 대한 얘기 같아서 여기서 emit은 "야기하기"라는 의미가 아니라 안정하지 않은 emit{Before,After}로 이해했습니다.


* **async_hooks**:
- 불안전을 야기하는 {Before,After}가 폐기 예정입니다. (Ali Ijaz Sheikh) [#18513](https://github.com/nodejs/node/pull/18513)
- PromiseWrap.parentId의 이름이 PromiseWrap.isChainedPromise로 변경되었습니다. (Ali Ijaz Sheikh) [#18633](https://github.com/nodejs/node/pull/18633)
Copy link
Contributor

Choose a reason for hiding this comment

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

이름을 ... 변경했습니다.는 어떤가요?

* **deps**:
- node-inspect가 1.11.3으로 업데이트되었습니다. (Jan Krems) [#18354](https://github.com/nodejs/node/pull/18354)
- ICU 60.2 bump (Steven R. Loomis) [#17687](https://github.com/nodejs/node/pull/17687)
- V8에 ScriptOrModule과 HostDefinedOptions가 소개되었습니다. (Jan Krems) [#16889](https://github.com/nodejs/node/pull/16889)
Copy link
Contributor

Choose a reason for hiding this comment

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

도입했습니다.는 어떤가요?

* **http2**:
- .createServer에 http 폴백 옵션이 추가되었습니다. (Peter Marton) [#15752](https://github.com/nodejs/node/pull/15752)
* **https**:
- tls.createSecureContext()에 남아있는 옵션이 Agent#getName()을 사용해 생성된 문자열에 추가되었습니다. 이는 https.request()가 옵션과 적절히 생성된 소켓을 승인하도록 허용합니다. (Jeff Principe) [#16402](https://github.com/nodejs/node/pull/16402)
Copy link
Contributor

Choose a reason for hiding this comment

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

tls.createSecureContext()에서 남은 옵션을 Agent#getName()로 생성한 문자열로 추가합니다. 이는 통해 https.request()가 옵션을 받고 적절하게 유일한 소켓을 생성할 수 있습니다. 는 어떤가요?

* **https**:
- tls.createSecureContext()에 남아있는 옵션이 Agent#getName()을 사용해 생성된 문자열에 추가되었습니다. 이는 https.request()가 옵션과 적절히 생성된 소켓을 승인하도록 허용합니다. (Jeff Principe) [#16402](https://github.com/nodejs/node/pull/16402)
* **inspector**:
- es 모듈을 위한 --inspect-brk (Guy Bedford) [#18194](https://github.com/nodejs/node/pull/18194)
Copy link
Contributor

Choose a reason for hiding this comment

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

--inspect-brk를 추가했습니다.는 어떤가요?

* **lib**:
- signal number를 사용해 프로세스를 중단할 수 있도록 허용합니다. (Sam Roberts) [#16944](https://github.com/nodejs/node/pull/16944)
* **module**:
- dynamic import를 가능하게 합니다. (Myles Borins) [#18387](https://github.com/nodejs/node/pull/18387)
Copy link
Contributor

Choose a reason for hiding this comment

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

활성화했습니다.
는 어떨까요?

@freenice12
Copy link
Contributor Author

리뷰 감사합니다. 모두 반영했습니다.
그리고 전에도 빌드는 했던것 같아요. 제 기억에는?

- 안전하지 않은 emit{Before,After}는 폐기 예정입니다. (Ali Ijaz Sheikh) [#18513](https://github.com/nodejs/node/pull/18513)
- PromiseWrap.parentId의 이름을 PromiseWrap.isChainedPromise로 변경했습니다. (Ali Ijaz Sheikh) [#18633](https://github.com/nodejs/node/pull/18633)
* **deps**:
- node-inspect가 1.11.3으로 업데이트되었습니다. (Jan Krems) [#18354](https://github.com/nodejs/node/pull/18354)
Copy link
Contributor

Choose a reason for hiding this comment

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

'node-inspect를 1.11.3으로 업데이트했습니다'는 어떻습니까?

- PromiseWrap.parentId의 이름을 PromiseWrap.isChainedPromise로 변경했습니다. (Ali Ijaz Sheikh) [#18633](https://github.com/nodejs/node/pull/18633)
* **deps**:
- node-inspect가 1.11.3으로 업데이트되었습니다. (Jan Krems) [#18354](https://github.com/nodejs/node/pull/18354)
- ICU 60.2 bump (Steven R. Loomis) [#17687](https://github.com/nodejs/node/pull/17687)
Copy link
Contributor

Choose a reason for hiding this comment

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

'ICU를 60.2로 업데이트했습니다.'는 어떻습니까?

* **deps**:
- node-inspect가 1.11.3으로 업데이트되었습니다. (Jan Krems) [#18354](https://github.com/nodejs/node/pull/18354)
- ICU 60.2 bump (Steven R. Loomis) [#17687](https://github.com/nodejs/node/pull/17687)
- V8에 ScriptOrModule과 HostDefinedOptions가 도입했습니다. (Jan Krems) [#16889](https://github.com/nodejs/node/pull/16889)
Copy link
Contributor

Choose a reason for hiding this comment

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

HostDefinedOptions가 → HostDefinedOptions를

- ICU 60.2 bump (Steven R. Loomis) [#17687](https://github.com/nodejs/node/pull/17687)
- V8에 ScriptOrModule과 HostDefinedOptions가 도입했습니다. (Jan Krems) [#16889](https://github.com/nodejs/node/pull/16889)
* **http**:
- `IncomingMessage`와 `ServerReponse`를 위한 옵션이 http.createServer()에 추가되었습니다. (Peter Marton) [#15752](https://github.com/nodejs/node/pull/15752)
Copy link
Contributor

Choose a reason for hiding this comment

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

`ServerReponse`를 → `ServerResponse`를

원문에 오타가 있네요. PR 만들어뒀습니다. nodejs/nodejs.org#1576

* **http2**:
- .createServer에 http 폴백 옵션이 추가되었습니다. (Peter Marton) [#15752](https://github.com/nodejs/node/pull/15752)
* **https**:
- tls.createSecureContext()에서 남은 옵션을 Agent#getName()로 생성한 문자열로 추가합니다. 이는 통해 https.request()가 옵션을 받고 적절하게 유일한 소켓을 생성할 수 있습니다. (Jeff Principe) [#16402](https://github.com/nodejs/node/pull/16402)
Copy link
Contributor

Choose a reason for hiding this comment

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

  • tls.createSecureContext()에서 남은 옵션을 Agent#getName()로 생성한 문자열로 추가합니다.

    tls.createSecureContext()에서 남은 옵션을 Agent#getName()이 생성한 문자열에 추가합니다.

  • 적절하게 유일한 소켓을 생성할 수 있습니다 → 유일한 소켓을 적절하게 생성할 수 있습니다

* **inspector**:
- es 모듈을 위한 --inspect-brk를 추가했습니다. (Guy Bedford) [#18194](https://github.com/nodejs/node/pull/18194)
* **lib**:
- signal number를 사용해 프로세스를 중단할 수 있도록 허용합니다. (Sam Roberts) [#16944](https://github.com/nodejs/node/pull/16944)
Copy link
Contributor

Choose a reason for hiding this comment

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

kill 자체는 시그널을 보내는 용도로 사용되고 프로세스를 중단하는 시그널만 있는 것은 아닌 것 같습니다.

'signal number를 사용해 프로세스에 시그널을 보낼 수 있습니다.'는 어떻습니까? kill을 영문으로 적는 방법도 있을 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

원문: allow process kill by signal number
제가 해당 커밋을 확인했을 때는 signal number를 통한 프로세스 중단에 관한 코드가 수정된것으로 보이는데요.
혹시 이전에는 signal number를 통한 프로세스 중단이 오류가 있어 이번에 수정된건 아닐까 추측해봅니다.

그래서 혹시
signal number를 사용해도 프로세스를 중단할 수 있도록 허용합니다.
는 어떨지 의견 부탁드립니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

코드를 보시면 constants[sig]를 통해서만 넘기던 signal을 바로 숫자로 넘길 수도 있게 바꾸었습니다. 그래서 이전에는 'SIGHUP', 'SIGINT' 등의 시그널 문자열을 넘겼다면 지금은 1, 2로 바로 넘길 수도 있게 되는 것입니다. 그런데 아래에 나열된 SIG...를 넘긴다고 해서 프로세스가 무조건 중단되는 것은 아닙니다. kill이라는 메서드는 해당 프로세스에 시그널을 보내는 역할을 수행하고, 만약 SIGINTSIGTERM 등의 특정 시그널을 받으면 프로세스가 종료하는 것입니다. SIGCONT 같은 경우엔 프로세스가 멈추었을 경우 다시 재개하도록 하는 역할을 하기도 합니다. 저는 원문의 'kill'을 process.kill 정도의 의미로 해석했습니다.

> os.constants.signals
{ SIGHUP: 1,
  SIGINT: 2,
  SIGQUIT: 3,
  SIGILL: 4,
  SIGTRAP: 5,
  SIGABRT: 6,
  SIGIOT: 6,
  SIGBUS: 10,
  SIGFPE: 8,
  SIGKILL: 9,
  SIGUSR1: 30,
  SIGSEGV: 11,
  SIGUSR2: 31,
  SIGPIPE: 13,
  SIGALRM: 14,
  SIGTERM: 15,
  SIGCHLD: 20,
  SIGCONT: 19,
  SIGSTOP: 17,
  SIGTSTP: 18,
  SIGTTIN: 21,
  SIGTTOU: 22,
  SIGURG: 16,
  SIGXCPU: 24,
  SIGXFSZ: 25,
  SIGVTALRM: 26,
  SIGPROF: 27,
  SIGWINCH: 28,
  SIGIO: 23,
  SIGINFO: 29,
  SIGSYS: 12 }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아. 제가 이해 못한 부분이 그곳이었군요.
시간을 빼앗아 죄송합니다.

말씀하신
signal number를 사용해 프로세스에 시그널을 보낼 수 있습니다.
로 수정하도록 하겠습니다(영문 kill은 저같은 사람에게 오해의 소지가 있을것 같습니다 ㅠㅠ).

Copy link
Contributor

Choose a reason for hiding this comment

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

아닙니다! 원문에 kill이 있다 보니 상당히 중의적으로 읽히네요.

- dynamic import를 활성화했습니다. (Myles Borins) [#18387](https://github.com/nodejs/node/pull/18387)
- dynamic import를 사용하실 수 있습니다. (Jan Krems) [#15713](https://github.com/nodejs/node/pull/15713)
* **n-api**:
- open/close 콜백 스코프에 메소드를 추가했습니다. (Michael Dawson) [#18089](https://github.com/nodejs/node/pull/18089)
Copy link
Contributor

Choose a reason for hiding this comment

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

메소드 → 메서드

@yous yous merged commit 5af0c48 into nodejs:master Mar 2, 2018
@yous yous removed the in review label Mar 2, 2018
@yous yous mentioned this pull request Mar 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants