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

src: alias BINARY to LATIN1 #7284

Merged
merged 1 commit into from
Jun 19, 2016
Merged

src: alias BINARY to LATIN1 #7284

merged 1 commit into from
Jun 19, 2016

Conversation

bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented Jun 13, 2016 •
edited
Loading

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/

@bnoordhuis bnoordhuis 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
Member Author

LATIN1 hasn't landed in a release yet.

@cjihrig
Copy link
Contributor

cjihrig commented Jun 13, 2016

LGTM

1 similar comment
@addaleax
Copy link
Member

LGTM

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

Needs to land for #7323

@bnoordhuis bnoordhuis mentioned this pull request Jun 17, 2016
Merged
4 tasks
@bnoordhuis
Copy link
Member Author

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

@bnoordhuis
Copy link
Member Author

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

@cjihrig
Copy link
Contributor

cjihrig commented Jun 17, 2016

CI was good minus a seemingly unrelated failure.

@bnoordhuis
src: alias BINARY to LATIN1
a92089b 
Make BINARY an alias for LATIN1 rather than a distinct enum value.

PR-URL: nodejs#7284
Refs: nodejs#7262
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@bnoordhuis bnoordhuis closed this Jun 19, 2016
@bnoordhuis bnoordhuis deleted the alias-binary-to-latin1 branch June 19, 2016 07:29
@bnoordhuis bnoordhuis merged commit a92089b into nodejs:master Jun 19, 2016
@addaleax addaleax mentioned this pull request Aug 8, 2016
Closed
addaleax pushed a commit to addaleax/node that referenced this pull request Aug 8, 2016
@bnoordhuis @addaleax
src: alias BINARY to LATIN1
da9bd2f 
Make BINARY an alias for LATIN1 rather than a distinct enum value.

PR-URL: nodejs#7284
Refs: nodejs#7262
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@addaleax addaleax mentioned this pull request Aug 8, 2016
Merged
@cjihrig cjihrig mentioned this pull request Aug 11, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@bnoordhuis @thefourtheye @cjihrig @addaleax @Fishrock123