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.

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

tools: fix license-builder.sh again for ICU #6068

Closed
wants to merge 1 commit into from

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Apr 5, 2016

Pull Request check-list

Please make sure to review and check all of these items:

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to CONTRIBUTING.md?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Affected core subsystem(s)

tools and doc only

Description of change

* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in nodejs#6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in nodejs#6065 is already
included here.
@srl295 srl295 added the tools Issues and PRs related to the tools directory. label Apr 5, 2016
@srl295
Copy link
Member Author

srl295 commented Apr 5, 2016

I filed IcuBug:12452 because I realized I made ICU's LICENSE file have a BOM, oops. The regex committed here should work regardless.

@mscdex mscdex added the i18n-api Issues and PRs related to the i18n implementation. label Apr 5, 2016
@Fishrock123
Copy link
Contributor

r=@rvagg

@rvagg
Copy link
Member

rvagg commented Apr 6, 2016

lgtm, seems to work. What does this one do: '1s/^[^a-zA-Z ]*ICU/ICU/'?

@srl295
Copy link
Member Author

srl295 commented Apr 6, 2016

Strips the BOM. Double, triple irony here. edit this is what the ICU bug was filed for above. This is my attempt to not make the regex platform specific.

@jasnell
Copy link
Member

jasnell commented Apr 7, 2016

LGTM

jasnell pushed a commit that referenced this pull request Apr 18, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Apr 18, 2016

Landed in 7097212

@jasnell jasnell closed this Apr 18, 2016
MylesBorins pushed a commit that referenced this pull request Apr 19, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Apr 20, 2016
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request Apr 25, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in nodejs#6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in nodejs#6065 is already
included here.

PR-URL: nodejs#6068
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request Apr 26, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 1, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jun 24, 2016
MylesBorins pushed a commit that referenced this pull request Jun 24, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 24, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n-api Issues and PRs related to the i18n implementation. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants