src: re-sort the symbol macros #24382
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nodejs-github-bot
added
the
c++
|
I don't see how this would need anything more than a ci lite to land, and perhaps it is a candidate for fast tracing? Not sure if we fast track based on triviality, or urgency. |
devsnek
approved these changes
Nov 15, 2018
|
code changes should always get the full ci. i have no opinion about the fast tracking beyond that |
refack
approved these changes
Nov 15, 2018
Urgency yes (regression and red CI fixers), and if the possible impact is minimal. Personally I'm not a big fan of fast tracking, but I won't block this on. |
cjihrig
approved these changes
Nov 15, 2018
fhinkel
approved these changes
Nov 16, 2018
addaleax
approved these changes
Nov 16, 2018
|
I’d be in favour of fast-tracking, since this PR is not complex and there’s a decent chance of generating merge conflicts that we could otherwise avoid. |
refack
added
the
author ready
The symbol macros were almost lexically sorted, but some were misplaced. PR-URL: nodejs#24382 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
refack
force-pushed
the
sort-env-symbols
branch
from
ffcd730
to
5f9b624
Compare
|
Pre-land extra CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/1631/ |
targos
pushed a commit
that referenced
this pull request
Nov 18, 2018
The symbol macros were almost lexically sorted, but some were misplaced. PR-URL: #24382 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
4 tasks
rvagg
pushed a commit
that referenced
this pull request
Nov 28, 2018
The symbol macros were almost lexically sorted, but some were misplaced. PR-URL: #24382 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
This was referenced Dec 7, 2018
|
@sam-github do you think this is worth trying to backport to |
|
Yes, I'll do it, because PRs on master that touch the sorted macros won't merge clean unless the 10.x macros are also sorted. Its trivial to backport, I'll just resort the macros that exist already in 10.x. |
4 tasks
|
v10.x backport: #25500 |
sam-github
added a commit
to sam-github/node
that referenced
this pull request
Feb 4, 2019
The symbol macros were almost lexically sorted, but some were misplaced. Backport-PR-URL: nodejs#25500 PR-URL: nodejs#24382 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
BethGriggs
pushed a commit
that referenced
this pull request
Feb 5, 2019
The symbol macros were almost lexically sorted, but some were misplaced. Backport-PR-URL: #25500 PR-URL: #24382 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Merged
rvagg
pushed a commit
that referenced
this pull request
Feb 28, 2019
The symbol macros were almost lexically sorted, but some were misplaced. Backport-PR-URL: #25500 PR-URL: #24382 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The symbol macros were almost lexically sorted, but some were misplaced.
I noticed when I added a few, then sorted the symbol block expecting the new ones to just go into the right place, which they did, but a dozen pre-existing ones moved around, too.
I think this could be fast-tracked, it should make no functional difference.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes