search

celo-org / governance

Governance Repository for Celo
Apache License 2.0
46 stars 68 forks source link

YAML Front Matter fixes and removal of HTML in CGP content #389

Closed jmrossy closed 8 months ago

jmrossy commented 8 months ago

The YAML front matter for most proposals had formatting problems, incorrect types, or other issues. Without proper formatting, apps cannot make use of data. In addition, some proposals contained HTML instead of just markup. In fact CGP 64 was entirely html and should not have been accepted in this format.

Summary of fixes:

jmrossy commented 8 months ago

@0xzoz @juanjgiraldoc @0xGoldo @ericnakagawa Any chance we can get a review on this? I realize it's a large PR but this is what was required to fix the inconsistencies. The upcoming Celo Station grant project will require good governance data to function :)

0xzoz commented 8 months ago

@0xzoz @juanjgiraldoc @0xGoldo @ericnakagawa Any chance we can get a review on this? I realize it's a large PR but this is what was required to fix the inconsistencies. The upcoming Celo Station grant project will require good governance data to function :)

Thanks for this! amazing work. one more review and we will merge

work-0xj4an commented 8 months ago

Thanks @jmrossy for doing this, But I am also quite confused about CGP 53. (Note the difference between the json files - & )_

This one inside the GGP-0053 Folder CGP-0053:

And for the the json of this proposal is this one: https://github.com/celo-org/governance/blob/main/CGPs/cgp-0053/cgp_0053.json

This one in the normal way: CGP-0053 proposal:

And for me the Json file is this one: https://github.com/celo-org/governance/blob/main/CGPs/cgp-0053/cgp-0053.json

jmrossy commented 8 months ago

@juanjgiraldoc Yes good catch about 0053. It seems that both 0053 CGPs were created around the same time. Javi created one correctly: https://github.com/celo-org/governance/pull/131/files Roman unfortunately did not, though his came first: https://github.com/celo-org/governance/pull/132/files

I'm not sure how to resolve this. Both were executed so we should keep both. I guess one should be re-numbered and we can include a note about the old number. WDYT?

Edit: I just noticed there's not CGP 66 so maybe Roman's 53 and become 66.

work-0xj4an commented 8 months ago

@jmrossy Maybe we can rename as CGP-0053 and CGP-0053A? wdyt because roman CGP-0053 PR date is May 13 2022 and the CGP-0065 PR is from Dec 1 2022 too far away...

jmrossy commented 8 months ago

@juanjgiraldoc In my opinion having the two unrelated PRs share a number (53, 53A) will imply they are related and probably create more confusion than the date gap. Also it will make it more difficult for apps like Celo Station to fetch and organize the proposal data when there are duplicates with the same CGP number.

Would you be okay with using a different number for the two? We could also use a new number if you prefer (instead of 66)

work-0xj4an commented 8 months ago

@jmrossy Understand, but this one is not the only one, don't know why they numbered those CGPs this way.

Anyway, what is best for the Celo Station, feel free to do it.

jmrossy commented 8 months ago

@juanjgiraldoc Yes those two P proposals do indeed cause problems in Celo Station so the app is ignoring them for now.

Shall I rename them to drop the P suffix? I can't find what it means. If you don't know either, then certainly the average community member won't know. So it's causing confusion and isn't worth the inconsistency. It's also possible it was a mistake that was copied once.

jmrossy commented 8 months ago

@juanjgiraldoc changes are done :)