- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 33.6k
doc: Update tools/icu/README.md #16939
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
Conversation
        
          
                tools/icu/README.md
              
                Outdated
          
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: not just Intl.* — it‘s also used for e.g. RegExp Unicode property escapes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mathiasbynens Would just using internationalization functionality instead of Intl functionality sound good to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that sounds good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(dramatic pause) 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| CI: https://ci.nodejs.org/job/node-test-pull-request-lite/10/ I would like to land this soon so will implement the change suggested by @mathiasbynens myself. It's been sitting around for almost a month now due to that tiny little thing. | 
| New Mini-CI (seemed like the old one failed?): https://ci.nodejs.org/job/node-test-commit-light/148/ | 
56d88de    to
    2343bd8      
    Compare
  
    2343bd8    to
    bd36a36      
    Compare
  
    bd36a36    to
    34cbce9      
    Compare
  
    - remove TODOs: the one about defaults has been addressed, and the one about testing is a work item that doesn't belong in a doc. - add some background information Fixes: #7843 PR-URL: #16939 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
34cbce9    to
    07712d1      
    Compare
  
    - remove TODOs: the one about defaults has been addressed, and the one about testing is a work item that doesn't belong in a doc. - add some background information Fixes: #7843 PR-URL: #16939 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
- remove TODOs: the one about defaults has been addressed, and the one about testing is a work item that doesn't belong in a doc. - add some background information Fixes: #7843 PR-URL: #16939 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
- remove TODOs: the one about defaults has been addressed, and the one about testing is a work item that doesn't belong in a doc. - add some background information Fixes: #7843 PR-URL: #16939 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
| Does this need to be backported? | 
- remove TODOs: the one about defaults has been addressed, and the one about testing is a work item that doesn't belong in a doc. - add some background information Fixes: nodejs#7843 PR-URL: nodejs#16939 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
- remove TODOs: the one about defaults has been addressed, and the one about testing is a work item that doesn't belong in a doc. - add some background information Fixes: #7843 PR-URL: #16939 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
- remove TODOs: the one about defaults has been addressed, and the one about testing is a work item that doesn't belong in a doc. - add some background information Fixes: #7843 PR-URL: #16939 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
- remove TODOs: the one about defaults has been addressed, and the one about testing is a work item that doesn't belong in a doc. - add some background information Fixes: #7843 PR-URL: #16939 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
addressed, and the one about testing is a work
item that doesn't belong in a doc.
Fixes: #7843
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)