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

feat: permits are generated from the calculation results and redirect to the permit claim page #8

Conversation

gentlementlegen
Copy link
Member

@gentlementlegen gentlementlegen commented Apr 11, 2024

Resolves #9

Depends on ubiquity-os/permit-generation#10

@gentlementlegen gentlementlegen changed the title Feat/permit generation feat: permits are generated from the calculation results and redirect to the permit claim page Apr 11, 2024
@gentlementlegen
Copy link
Member Author

gentlementlegen commented Apr 11, 2024

Changes in this PR:

The module is now capable of generating permits for the user rewards. It creates the permit and links it inside each user reward information. Last successful run: https://github.com/gentlementlegen/conversation-rewards/actions/runs/8642076491/job/23692567706#step:3:85 You can click on the links to open the reward page.

Later on I believe we want this logic to be moved to the kernel. As they don't communicate yet I thought it'd better to have a first version that is fully autonomous.

Also it currently has a 5 digits precision for rewards, which is maybe not wanted. Let me know if you wanna round number to fewer digits.

Required environment variables to run properly:

  • OPENAI_API_KEY
  • X25519_PRIVATE_KEY
  • SUPABASE_KEY
  • SUPABASE_URL

And Knip failing as usual for the same reasons 🤡 and Jest because env variables are not set yet.

@gentlementlegen gentlementlegen marked this pull request as ready for review April 11, 2024 05:51
@gitcoindev
Copy link
Contributor

Changes in this PR:

The module is now capable of generating permits for the user rewards. It creates the permit and links it inside each user reward information. Last successful run: https://github.com/gentlementlegen/conversation-rewards/actions/runs/8642076491/job/23692567706#step:3:85 You can click on the links to open the reward page.

Later on I believe we want this logic to be moved to the kernel. As they don't communicate yet I thought it'd better to have a first version that is fully autonomous.

Also it currently has a 5 digits precision for rewards, which is maybe not wanted. Let me know if you wanna round number to fewer digits.

Required environment variables to run properly:

  • OPENAI_API_KEY
  • X25519_PRIVATE_KEY
  • SUPABASE_ANON_KEY
  • SUPABASE_URL

And Knip failing as usual for the same reasons 🤡 and Jest because env variables are not set yet.

Good catch! I will fix this repository Knip asap, also review the pr today.

@gitcoindev
Copy link
Contributor

Knip config fixed at #11 I am reviewing this pull request now.

Copy link
Contributor

@gitcoindev gitcoindev left a comment

Choose a reason for hiding this comment

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

I added a few comments to clarify, but overall the pull request is very useful. Good job! I think it would be useful to add documentation about releases, perhaps an automated label in README.md would be fine and add the documentation on generating rewards on different network ids.

.github/workflows/release-please.yml Show resolved Hide resolved
src/parser/permit-generation-module.ts Show resolved Hide resolved
.github/workflows/release-please.yml Show resolved Hide resolved
@0x4007 0x4007 removed their request for review April 13, 2024 07:19
@0x4007
Copy link
Member

0x4007 commented Apr 13, 2024

Why five digits of precision? The industry standard is 18.

@molecula451
Copy link

looks like 2 npm packages are in conflict @gentlementlegen

@gentlementlegen
Copy link
Member Author

Why five digits of precision? The industry standard is 18.

I am using 18 digits, I meant that the final result can potentially be 4.1654627 WXDAI for example, and I don't recall having such results with the version we have currently for calculating the total.

gentlementlegen and others added 5 commits April 15, 2024 13:21
# Conflicts:
#	.github/workflows/main.yml
#	action.yml
#	package.json
#	src/parser/permit-generation-module.ts
#	src/types/env.d.ts
#	yarn.lock
Copy link
Contributor

@whilefoo whilefoo left a comment

Choose a reason for hiding this comment

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

code looks good

.github/workflows/jest-testing.yml Outdated Show resolved Hide resolved
src/types/env.d.ts Outdated Show resolved Hide resolved
src/parser/permit-generation-module.ts Outdated Show resolved Hide resolved
src/parser/permit-generation-module.ts Show resolved Hide resolved
src/parser/permit-generation-module.ts Show resolved Hide resolved
@gentlementlegen gentlementlegen merged commit dd6b3ee into ubiquity-os-marketplace:development Apr 17, 2024
1 of 2 checks passed
@gentlementlegen gentlementlegen deleted the feat/permit-generation branch April 17, 2024 06:35
@gentlementlegen
Copy link
Member Author

I noticed now that cspell checks are failing (due to unkown word SUPABASE) but I notice just now because this workflow does not run on PRs.
https://github.com/ubiquibot/conversation-rewards/actions/runs/8717404761/job/23912565868#step:5:62

Might wanna consider have it rolling on pull_request as well. I don't know if this was a choice due to the secrets not being populated.

@0x4007
Copy link
Member

0x4007 commented Apr 17, 2024

You can add words to cspell config. Sure you should run on pulls.

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.

Generate permits with urls for each rewarded participant
5 participants