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

ICU-21427 Don't ignore already checked-in files under "tools/cldr/lib". #1500

Merged
merged 1 commit into from
Jan 20, 2021

Conversation

jefgen
Copy link
Member

@jefgen jefgen commented Dec 6, 2020

I recently ran into trouble trying to mirror/copy files from one ICU clone to another clone due to there being already checked-in files that were ignored by the .gitignore file under tools/cldr

The problem is that it excludes everything under the lib folder:

/lib/**

However, there are 3 checked in files in that folder which likely shouldn't be gitignored:

install-cldr-jars.txt
README.txt
pom.xml

The README.txt file is already excluded by the .gitignore file:

!/lib/README.txt

We should likely add the other two files to the .gitignore file as well.

Checklist

daniel-ju
daniel-ju previously approved these changes Dec 7, 2020
markusicu
markusicu previously approved these changes Jan 19, 2021
Copy link
Member

@markusicu markusicu left a comment

Choose a reason for hiding this comment

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

lgtm

srl295
srl295 previously approved these changes Jan 19, 2021
Copy link
Member

@srl295 srl295 left a comment

Choose a reason for hiding this comment

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

#1533 has additional comments

@jefgen
Copy link
Member Author

jefgen commented Jan 19, 2021

#1533 has additional comments

Good point, I'll add the comments from that, set this as co-authored by @gvictor, and then force push in a moment.

Co-authored-by: Victor Chang <vichang@google.com>
@jefgen jefgen dismissed stale reviews from srl295, markusicu, and daniel-ju via 4ac4773 January 19, 2021 22:52
@jefgen jefgen force-pushed the user/jefgen/ICU-21427_gitignore branch from 5f5a5c5 to 4ac4773 Compare January 19, 2021 22:52
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • tools/cldr/.gitignore is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jefgen jefgen requested a review from gvictor January 19, 2021 22:52
@jefgen jefgen merged commit dc5ab7a into unicode-org:master Jan 20, 2021
@jefgen jefgen deleted the user/jefgen/ICU-21427_gitignore branch January 20, 2021 01:08
jefgen added a commit to microsoft/icu that referenced this pull request Jan 20, 2021
jefgen added a commit to microsoft/icu that referenced this pull request Jan 20, 2021
This change cherry-picks two tickets from the upstream ICU repo that were merged after the ICU 68.2 release.

- ICU-21427 Don't ignore already checked-in files under "tools/cldr/lib"
- ICU-21118 check that dst and src are not null in uprv_memcpy

The first one I had already done with #52, so this is mostly just making an update for the comments and to record it in the `changelog.md` file.

The second one was fixed by @erik0686, and prevents undefined behavior in `ultag_parse`.

CP: ICU-21118 check that dst and src are not null in uprv_memcpy
https://unicode-org.atlassian.net/browse/ICU-21118
unicode-org/icu#1489

CP: ICU-21427 Don't ignore already checked-in files under "tools/cldr/lib"
https://unicode-org.atlassian.net/browse/ICU-21427
unicode-org/icu#1500

Update changelog.md for cherry-picked tickets.
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.

5 participants