Skip to content

Conversation

@bnoordhuis
Copy link
Member

@bnoordhuisbnoordhuis commented Jun 13, 2016

Make BINARY an alias for LATIN1 rather than a distinct enum value.

Refs: #7262

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

@bnoordhuisbnoordhuis added c++ Issues and PRs that require attention from people who are familiar with C++. test Issues and PRs related to the tests. lib / src Issues and PRs related to general changes in the lib or src directory. labels Jun 13, 2016
@thefourtheye
Copy link
Contributor

LGTM. We are redefining LATIN1 with a different value. Isn't this going to be a major change?

@bnoordhuis
Copy link
MemberAuthor

LATIN1 hasn't landed in a release yet.

@cjihrig
Copy link
Contributor

LGTM

1 similar comment
@addaleax
Copy link
Member

LGTM

@Fishrock123Fishrock123 mentioned this pull request Jun 17, 2016
@Fishrock123
Copy link
Contributor

Needs to land for #7323

@bnoordhuis
Copy link
MemberAuthor

This shouldn't land in any of the release lines. I've added the tags.

@bnoordhuis
Copy link
MemberAuthor

New CI, previous one seems to have been cancelled: https://ci.nodejs.org/job/node-test-pull-request/3013/

@cjihrig
Copy link
Contributor

CI was good minus a seemingly unrelated failure.

Make BINARY an alias for LATIN1 rather than a distinct enum value. PR-URL: nodejs#7284 Refs: nodejs#7262 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
@bnoordhuisbnoordhuisforce-pushed the alias-binary-to-latin1 branch from 7b40780 to a92089bCompareJune 19, 2016 07:29
@bnoordhuisbnoordhuis deleted the alias-binary-to-latin1 branch June 19, 2016 07:29
@bnoordhuisbnoordhuis merged commit a92089b into nodejs:masterJun 19, 2016
@addaleaxaddaleax mentioned this pull request Aug 8, 2016
addaleax pushed a commit to addaleax/node that referenced this pull request Aug 8, 2016
Make BINARY an alias for LATIN1 rather than a distinct enum value. PR-URL: nodejs#7284 Refs: nodejs#7262 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
@cjihrigcjihrig mentioned this pull request Aug 11, 2016
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.lib / srcIssues and PRs related to general changes in the lib or src directory.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@bnoordhuis@thefourtheye@cjihrig@addaleax@Fishrock123