Conversation
|
The changes in Emoji.java are identical to what I had on the branch I sent you and I was setting -Demoji-beta. What changed? |
|
Is it because I also changed VersionToAge.java? Because I thought I was supposed to… |
| * You need a command line variable to generate either the beta version (not yet released) or the abbreviated version (see below). | ||
| * <p>Example: -Demoji-beta |
There was a problem hiding this comment.
The emoji-beta option is already mentioned on https://github.com/unicode-org/unicodetools/blob/main/docs/emoji/index.md
Wouldn't it be better to update that and point from here to there?
Note that there are several docs pages under emoji/ : https://github.com/unicode-org/unicodetools/tree/main/docs/emoji
There was a problem hiding this comment.
Please set your editor to write spaces, not tabs, and fix this comment.
|
As I mentioned yesterday, these changes seem like a subset of what I had done on my ned/emoji_15_draft branch so I have to ask: in the generated emoji-data.txt, are you seeing |
|
Sorry for being so uncommunicative; I'm overbooked right now. Will have a
chance to look at this today. I think there is a switch somewhere to
generate the beta data files; it might be key'ed off of the status of the
candidates (eg provisional v draft) in the candidateData file, but I have
to check. Will have some time later today.
…On Tue, May 3, 2022 at 2:21 PM Ned Holbrook ***@***.***> wrote:
As I mentioned yesterday, these changes seem like a subset of what I had
done on my ned/emoji_15_draft branch so I have to ask: in the generated
emoji-data.txt, are you seeing E15.0? and does emoji-test.txt contain
15.0 emoji?
—
Reply to this email directly, view it on GitHub
<#232 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACJLEMCRJOJOTQJ7CLJN3YLVIGKFZANCNFSM5U4FKVFQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Made modifications to get the files to build. Updated the emoji/index.md. Still need to build emoji-test.txt, but wanted to get these in first. There is a related PR in the emoji repo. |
nedley
left a comment
nedley
left a comment
There was a problem hiding this comment.
Thanks for taking the time to flesh this out. Having the 15.0 additions show up as E0.0 is a problem for the beta but we could fix those by hand if time is running low.
| @@ -0,0 +1,126 @@ | |||
| package org.unicode.propstest; | |||
There was a problem hiding this comment.
Is this file required for the E15.0 data?
|
|
||
| public static <T> void printData() throws IOException { | ||
| printData(EmojiDataSourceCombined.EMOJI_DATA); | ||
| printData(EmojiDataSourceCombined.EMOJI_DATA_BETA); |
There was a problem hiding this comment.
I’m assuming this was the missing piece?
| @@ -78,6 +98,7 @@ public class Emoji { | |||
| public static final VersionInfo VERSION0_5 = VersionInfo.getInstance(0,5,2); | |||
|
|
|||
| // ALSO fix VersionToAge.java! | |||
There was a problem hiding this comment.
Is this comment still relevant?
|
@macchiati I will approve your current PRs to not hold things up but at this point we still have issues that are probably blockers for the beta release: charts aside, they are the wrong ages and an incomplete emoji-test.txt. Given your limited availability next week, my proposal is that I make the necessary fixes by hand before trying to debug the tooling but if you know where the problem lies then I will obviously defer to you. |
|
After scrutinizing the updated comments in the code, it sounds like the emoji-test.txt will be fixed after copying the generated files and re-running the tool? |
…update the instructions)
|
Yes, the E0.0 and missing values in the test are fixed by running the tool again. There is one extra step that I now document in the index.md. See how it looks now. Haven't tracked down the X issue in the charts yet, but I think this part should be good enough to check in (after checking, of course). And will fix the X later today. |
|
@nedley @macchiati sorry for not noticing this before -- but the data files are in the wrong place! You should also have populated the files in https://github.com/unicode-org/unicodetools/tree/main/unicodetools/data/ucd/dev/emoji Please fix! |
|
I have sent PR #248 for review to move the emoji files to where they belong, sync the duplicated files, and update the readmes. |
3 participants
This should work now. You have to supply an environment variable to generate the beta. Added more documentation to the top of Emoji.java.