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

retrofit files with ICU4X license header #326

Merged
merged 2 commits into from
Oct 12, 2020

Conversation

echeran
Copy link
Contributor

@echeran echeran commented Oct 11, 2020

Applied to *.rs, *.toml, and *.yml files using the following commands, when run at the root of the project:

find . -name "*.rs" | egrep -v "deletemetest" | ruby -lane 'file=$F[0]; puts "sed -i " + 39.chr + "1i // This file is part of ICU4X. For terms of use, please see the file\\n// called LICENSE at the top level of the ICU4X source tree\\n// (online at: https://github.com/unicode-org/icu4x/blob/master/LICENSE )." + 39.chr + " " +file' | sh
find . -name "*.toml" | egrep -v "deletemetest" | ruby -lane 'file=$F[0]; puts "sed -i " + 39.chr + "1i # This file is part of ICU4X. For terms of use, please see the file\\n# called LICENSE at the top level of the ICU4X source tree\\n# (online at: https://github.com/unicode-org/icu4x/blob/master/LICENSE )." + 39.chr + " " +file' | sh
find . -name "*yml" | egrep -v "deletemetest" | ruby -lane 'file=$F[0]; puts "sed -i " + 39.chr + "1i # This file is part of ICU4X. For terms of use, please see the file\\n# called LICENSE at the top level of the ICU4X source tree\\n# (online at: https://github.com/unicode-org/icu4x/blob/master/LICENSE )." + 39.chr + " " +file' | sh

cc @hsivonen

Closes #255 .

@echeran echeran requested review from zbraniecki and sffc October 11, 2020 03:41
@echeran echeran requested review from nciric and a team as code owners October 11, 2020 03:41
zbraniecki
zbraniecki previously approved these changes Oct 11, 2020
Manishearth
Manishearth previously approved these changes Oct 11, 2020
nciric
nciric previously approved these changes Oct 11, 2020
Copy link
Contributor

@nciric nciric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question of URL - is /blob/ expected part?

@echeran
Copy link
Contributor Author

echeran commented Oct 11, 2020

@nciric Yeah, it is, at least it works: https://github.com/unicode-org/icu4x/blob/master/LICENSE That's how Github creates links to files in the repo working tree. Go figure. It trips me up all the time.

@echeran
Copy link
Contributor Author

echeran commented Oct 11, 2020

Can someone help explain the formatter errors? For some reason, adding in these header comments causes cargo fmt to fail. I can't merge until this is fixed:

$ cargo fmt
error: attributes are not yet allowed on `if` expressions
  --> /usr/local/google/home/elango/oss/icu4x/components/datetime/examples/work_log.rs:24:5
   |
24 |     #[cfg(debug_assertions)]
   |     ^^^^^^^^^^^^^^^^^^^^^^^^

error: attributes are not yet allowed on `if` expressions
  --> /usr/local/google/home/elango/oss/icu4x/components/plurals/examples/elevator_floors.rs:12:5
   |
12 |     #[cfg(debug_assertions)]
   |     ^^^^^^^^^^^^^^^^^^^^^^^^

error: attributes are not yet allowed on `if` expressions
  --> /usr/local/google/home/elango/oss/icu4x/components/plurals/examples/unread_emails.rs:12:5
   |
12 |     #[cfg(debug_assertions)]
   |     ^^^^^^^^^^^^^^^^^^^^^^^^

@zbraniecki
Copy link
Member

Is it possible that it triggers a cache with an older rust?

@echeran
Copy link
Contributor Author

echeran commented Oct 11, 2020

Is it possible that it triggers a cache with an older rust?

Good question, but it looks like we already removed caching, and have a note with the issue number for re-including caching at some point in the future.

@zbraniecki
Copy link
Member

I pulled your branch and that's what it did for me:

/home/zbraniecki/projects/icu4x/(HEAD)> rustc --version
rustc 1.47.0 (18bf6b4f0 2020-10-07)
/home/zbraniecki/projects/icu4x/(HEAD)> cargo fmt
/home/zbraniecki/projects/icu4x/(HEAD)> git st
HEAD detached at elango/license-header-retrofit
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   components/icu4x/src/lib.rs

no changes added to commit (use "git add" and/or "git commit -a")
/home/zbraniecki/projects/icu4x/(HEAD)> git diff
diff --git a/components/icu4x/src/lib.rs b/components/icu4x/src/lib.rs
index f3ef531..7baa60c 100644
--- a/components/icu4x/src/lib.rs
+++ b/components/icu4x/src/lib.rs
@@ -1,4 +1,3 @@
 // This file is part of ICU4X. For terms of use, please see the file
 // called LICENSE at the top level of the ICU4X source tree
 // (online at: https://github.com/unicode-org/icu4x/blob/master/LICENSE ).
-
/home/zbraniecki/projects/icu4x/(HEAD)> 

is there a chance that this is the issue?

sffc
sffc previously approved these changes Oct 12, 2020
@echeran echeran dismissed stale reviews from sffc, nciric, Manishearth, and zbraniecki via f8fc7b6 October 12, 2020 16:16
@echeran
Copy link
Contributor Author

echeran commented Oct 12, 2020

Yep, removing that newline fixed the formatter check. The formatter check is happy. I now just need a rubber stamp.

@echeran echeran merged commit e1e418e into unicode-org:master Oct 12, 2020
@echeran echeran deleted the license-header-retrofit branch June 17, 2021 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fill in MIT copyright notices before first release
5 participants