CLDR-16499 Prepare to Require a CLA for contributions #3653

srl295 · 2024-04-23T00:48:18Z

For v46, we do NOT require a CLA, but leave the code in place for the future

property REQUIRE_CLA off by default, if on it will enable the flow
no popups or menu items visible to users
enables loading .md files to the front end and as assets
improve cldrClient.getClient() to cache, speedup repeated calls
add notice for synchronizing webpack configuration
APIs for sign/revoke/read CLA
web flow for signing CLA and handling various cases
This PR completes the ticket.

ALLOW_MANY_COMMITS=true

- new claSigned bit must be true to allow writing - store CLAs as JSON blobs in user prefs - API to read/update/revoke CLA - static list of signatory orgs - new menu item for CLA Other improvements/ - store JSON in the user settings data - load .md file via webpack - display markdown using marked (same as the error/wikimedia abstracts) - fix the client so that it doesn't hit the OpenAPI spec multiple times

jira-pull-request-webhook · 2024-04-23T17:31:20Z

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

srl295 · 2024-04-26T14:09:26Z

@btangmu I'll fix this so that the unit tests don't need to sign a CLA !

I'm just wondering if for local testing we want a switch that auto-signs the CLA? But then again, if you create a vetter in a signing org such as Apple Google Microsoft then it won't ask for a CLA.

srl295 · 2024-04-26T15:32:40Z

@btangmu i think this could get any feedback from you if you want to provide it, knowing we still need to review the text and the overall process. But let me know if you see any structural issues.

btangmu

remove the commented-out code change for MainMenu.vue?

I made a couple other nit-picky comments (which I don't insist on), otherwise, it all looks great

tools/cldr-apps/js/src/views/MainMenu.vue

tools/cldr-apps/js/src/views/SignCla.vue

srl295 · 2024-05-30T16:24:31Z

remove the commented-out code change for MainMenu.vue?

I made a couple other nit-picky comments (which I don't insist on), otherwise, it all looks great

the menu item was removed because it's not relevant while CLAs are not required. I've changed it to being driven by a constant.

srl295 · 2024-05-30T16:24:50Z

@btangmu ready to review for merging and mothballing.

btangmu

Looks great, except I suggest renaming "signed" to "dateSigned", since it's a Date, not a boolean (unless I still misunderstand), and fixing the front-end comment saying it's boolean

I've left a few comments/questions I had while studying the PR, even though I was eventually able to answer them for myself. I think it would help to add some clarifying comments for maintainability. On the front end, "user.claSigned" really means the user has signed OR doesn't need to because DO_NOT_REQURE_CLA

btangmu · 2024-05-30T20:05:05Z

tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/UserAPI.java

@@ -39,6 +46,7 @@ public UserInfo(int userid) {
            this.org = null;
            this.emailHash = null;
            this.name = "User #" + userid;
+            this.claSigned = false; // TODO


TODO should have ticket link; and it's not clear what needs to be done, if anything; false makes sense for a logged-out user

btangmu · 2024-05-30T20:19:04Z

tools/cldr-apps/js/src/esm/cldrCla.mjs

+ * @property {string} name
+ * @property {boolean} unauthorized true if cla load failed
+ * @property {boolean} readonly true if cla may not be modified
+ * @property {boolean} signed true if signed, always true when returned from getCla()


on the back end, it looks like "signed" is a Date rather than boolean?

good catch, that should be the date signed.

should that still be changed to a date, then?

btangmu · 2024-05-30T20:20:58Z

tools/cldr-apps/src/main/java/org/unicode/cldr/web/ClaSignature.java

+    public boolean corporate; // signed as corporate
+
+    @Schema(required = false)
+    public Date signed;


Date not boolean, but front-end comments say it's boolean

btangmu · 2024-05-30T20:22:14Z

tools/cldr-apps/src/main/java/org/unicode/cldr/web/UserRegistry.java

+            } else if (ClaSignature.DO_NOT_REQURE_CLA
+                    && !config.getProperty("REQUIRE_CLA", false)) {
+                // no CLA needed unless CLDR_NEED_CLA is true.
+                return new ClaSignature("DO_NOT_REQURE_CLA");


this constructor presumably sets signed = true or signed = Date... OK, I've confirmed this suspicion

btangmu · 2024-05-30T20:27:46Z

tools/cldr-apps/src/main/java/org/unicode/cldr/web/ClaSignature.java

+        this.name = "Testing CLA: " + string;
+        this.employer = "Testing CLA";
+        this.corporate = true;
+        this.signed = new Date(0);


OK, here's the constructor used for DO_NOT_REQURE_CLA, setting signed = Date, which is treated as true

btangmu · 2024-05-30T20:31:19Z

tools/cldr-apps/js/src/views/MainHeader.vue

@@ -153,6 +161,10 @@ export default {
        this.voteCountMenu = null;
        this.voteLevelChanged = 0;
      }
+
+      // only need CLA if logged in
+      this.needCla = !!user && !user.claSigned;


I'm guessing that if DO_NOT_REQURE_CLA is true on the back end, then user.claSigned will get true (or truthy Date object?), even though that's sort of a fiction... OK, I've confirmed that

srl295 · 2024-09-04T16:45:27Z

@btangmu I'll fixup this one and get it in. It has a number of general improvements.

srl295 · 2024-09-06T14:51:27Z

@btangmu @macchiati this is ready to go in. it is disabled by default, but gets all the mechanism for requiring a CLA in place (just pending wording changes)

macchiati

Sgtm. Tom should review details

srl295 · 2024-09-06T17:04:45Z

@macchiati Tom had previously approved, just had merge conflicts.

srl295 force-pushed the cldr-16499/cla branch from 5ddf63a to 5f502b4 Compare April 23, 2024 17:31

CLDR-16499 synchronize .md between the webpack configs

f146c55

CLDR-16499 CLA skip when in UNITTEST

99a5e8e

srl295 self-assigned this Apr 26, 2024

srl295 requested a review from btangmu April 26, 2024 15:31

srl295 added 3 commits April 29, 2024 13:48

CLDR-16499 Update cla.md

8e0c21b

CLDR-16499 merge from main

f490498

CLDR-16499 CLA refactor, prepare to mothball

1d94fd0

btangmu requested changes May 10, 2024

View reviewed changes

tools/cldr-apps/js/src/views/MainMenu.vue Outdated Show resolved Hide resolved

tools/cldr-apps/js/src/views/SignCla.vue Show resolved Hide resolved

tools/cldr-apps/js/src/views/SignCla.vue Show resolved Hide resolved

srl295 added 2 commits May 30, 2024 10:18

CLDR-16499 merge from main

6b29eec

CLDR-16499 CLA: update docs, off by default

2ea59c6

srl295 changed the title ~~CLDR-16499 require a CLA for contributions~~ CLDR-16499 Prepare to Require a CLA for contributions May 30, 2024

CLDR-16499 CLA: refactor client code out per code review

3c6b88c

srl295 marked this pull request as ready for review May 30, 2024 16:23

srl295 requested a review from btangmu May 30, 2024 16:24

btangmu previously approved these changes May 30, 2024

View reviewed changes

srl295 mentioned this pull request Sep 5, 2024

CLDR-14913 Add Download XML button to Survey Tool #4010

Merged

1 task

CLDR-16499 merge from main

ad081a6

srl295 dismissed btangmu’s stale review via ad081a6 September 5, 2024 22:18

srl295 requested review from btangmu and macchiati September 5, 2024 22:19

macchiati approved these changes Sep 6, 2024

View reviewed changes

srl295 merged commit 2ebe44f into unicode-org:main Sep 6, 2024
13 checks passed

srl295 deleted the cldr-16499/cla branch September 6, 2024 17:04

haytenf pushed a commit to haytenf/cldr that referenced this pull request Sep 17, 2024

CLDR-16499 Prepare to Require a CLA for contributions (unicode-org#3653)

204bada

conradarcturus pushed a commit that referenced this pull request Sep 25, 2024

CLDR-16499 Prepare to Require a CLA for contributions (#3653)

d334c80

Redwoodtj mentioned this pull request Oct 13, 2024

[Snyk] Security upgrade swagger-client from 3.18.5 to 3.29.4 Redwoodtj/cldr#26

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLDR-16499 Prepare to Require a CLA for contributions #3653

CLDR-16499 Prepare to Require a CLA for contributions #3653

srl295 commented Apr 23, 2024 •

edited

Loading

jira-pull-request-webhook bot commented Apr 23, 2024

srl295 commented Apr 26, 2024

srl295 commented Apr 26, 2024

btangmu left a comment

srl295 commented May 30, 2024

srl295 commented May 30, 2024

btangmu left a comment

btangmu May 30, 2024

btangmu May 30, 2024

srl295 May 30, 2024

btangmu Sep 6, 2024

btangmu May 30, 2024

btangmu May 30, 2024

btangmu May 30, 2024

btangmu May 30, 2024

srl295 Sep 6, 2024

srl295 commented Sep 4, 2024

srl295 commented Sep 6, 2024

macchiati left a comment

srl295 commented Sep 6, 2024

CLDR-16499 Prepare to Require a CLA for contributions #3653

CLDR-16499 Prepare to Require a CLA for contributions #3653

Conversation

srl295 commented Apr 23, 2024 • edited Loading

jira-pull-request-webhook bot commented Apr 23, 2024

srl295 commented Apr 26, 2024

srl295 commented Apr 26, 2024

btangmu left a comment

Choose a reason for hiding this comment

srl295 commented May 30, 2024

srl295 commented May 30, 2024

btangmu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srl295 commented Sep 4, 2024

srl295 commented Sep 6, 2024

macchiati left a comment

Choose a reason for hiding this comment

srl295 commented Sep 6, 2024

srl295 commented Apr 23, 2024 •

edited

Loading