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

Rewards for simplifying/cleaning code #33

Open
0x4007 opened this issue Aug 1, 2024 · 20 comments
Open

Rewards for simplifying/cleaning code #33

0x4007 opened this issue Aug 1, 2024 · 20 comments

Comments

@0x4007
Copy link
Member

0x4007 commented Aug 1, 2024

I was working on this pull and thought it could be cool to have a small reward for simplifying the codebase.

Diff stats:

+5426
-6760

A simple formula could be to divide by 10 then subtract. i.e. 676 - 542.6 = $133.4

With our strict linter rules we should be able to avoid BS refactors.

Rationale

Generally codebase maintainers should try and keep the code clean and simple. This offers a direct financial incentive for everybody to do so. Our task oriented system is not very compatible with the continuous nature of code simplification and reduction, so this plugin addresses that.

Filters

We can use filename regex to make specific types of files eligible such as *.ts, *.sol

Remark

This is intended to be stacked on regular task related pulls as well (it doesn't need to be a dedicated refactor pull.)

@gentlementlegen
Copy link
Member

Sounds a bit dangerous if we don't ignore some files, we might need some filters.

@ishowvel
Copy link

Does this really have to be a full on plugin, from what i see adding this onto the assignee payout logic would be way more simpler

Copy link

! Error: HttpError: Resource not accessible by integration - https://docs.github.com/rest/issues/comments#create-an-issue-comment

@0x4007
Copy link
Member Author

0x4007 commented Oct 31, 2024

Does this really have to be a full on plugin, from what i see adding this onto the assignee payout logic would be way more simpler

It does seem simpler I agree but it's against our philosophy for plugins. They should all be separate plugins.

It will make more sense when we have our SDK functional and when we centralize payments to be aggregating "payment requests" from plugins.

@gentlementlegen @whilefoo what should we do to also render rewards from this one? It doesn't seem to make sense to include in conversation rewards but otherwise we have to build all the centralized payments infrastructure and requests.

@ishowvel
Copy link

Does this really have to be a full on plugin, from what i see adding this onto the assignee payout logic would be way more simpler

It does seem simpler I agree but it's against our philosophy for plugins. They should all be separate plugins.

It will make more sense when we have our SDK functional and when we centralize payments to be aggregating "payment requests" from plugins.

@gentlementlegen @whilefoo what should we do to also render rewards from this one? It doesn't seem to make sense to include in conversation rewards but otherwise we have to build all the centralized payments infrastructure and requests.

Until the centralized payment infrastructure gets built, I propose adding this logic on to the rewards plugin as it will incentivize and motivate contributors to solve more issues

Copy link

! Error: HttpError: Resource not accessible by integration - https://docs.github.com/rest/issues/comments#create-an-issue-comment

@whilefoo
Copy link

whilefoo commented Nov 1, 2024

@gentlementlegen @whilefoo what should we do to also render rewards from this one? It doesn't seem to make sense to include in conversation rewards but otherwise we have to build all the centralized payments infrastructure and requests

I think it's fine to temporarily add it to conversation-rewards until we implement permit requests.
The problem though is that if each plugin posts a comment about its rewards it becomes too noisy but if we let the kernel sum the rewards and post it then we lose metadata for example 'conversation-rewards' includes detailed information about rewards

Copy link

! Error: HttpError: Resource not accessible by integration - https://docs.github.com/rest/issues/comments#create-an-issue-comment

@0x4007
Copy link
Member Author

0x4007 commented Nov 1, 2024

then we lose metadata for example 'conversation-rewards' includes detailed information about rewards

We just need to be mindful of comment character limits. Otherwise we can absolutely embed the metadata within each "field" as I mentioned in the other discussion we had about consolidating output comments from plugins.

Copy link

! Error: HttpError: Resource not accessible by integration - https://docs.github.com/rest/issues/comments#create-an-issue-comment

@ishowvel
Copy link

ishowvel commented Nov 2, 2024

Shouldnt this issue be moved to text-convo-rewards now?

Copy link

! Error: HttpError: Resource not accessible by integration - https://docs.github.com/rest/issues/comments#create-an-issue-comment

@gentlementlegen
Copy link
Member

gentlementlegen commented Nov 2, 2024

I will move it to conversation rewards in the meantime then.


Edit: we cannot transfer because it is a different organization I suppose, so it's fine to keep it here, just link the pull request to this issue from text-conversation-rewards repository.

@ishowvel
Copy link

ishowvel commented Nov 2, 2024

Also on a side note I have a general question, why does the text convo plugin handle all rewards and not just text convo rewards, why does it handle things like issue specification and assignee task completion reward

@ishowvel
Copy link

ishowvel commented Nov 2, 2024

It would make the plugin simpler if the logic could be divided further into multiple plugins

@ishowvel
Copy link

ishowvel commented Nov 3, 2024

/start

Copy link

Warning! This task was created over 94 days ago. Please confirm that this issue specification is accurate before starting.
Deadline Sun, Nov 3, 12:22 PM UTC
Beneficiary 0x340D8d2bd82dEb4f485623453c9F6ad307e6B027

Tip

  • Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
  • Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
  • Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

@ishowvel
Copy link

ishowvel commented Nov 3, 2024

i dont have enough time to work on this so i will just point out instructions for future devs to work on this

@ishowvel
Copy link

ishowvel commented Nov 3, 2024

/stop

@ishowvel
Copy link

ishowvel commented Nov 3, 2024

  1. Add a new type of reward to denote the amount of lines simplified in here
  2. in the permit generation make a function to calculate the reward kinda like
async function getSimplifyReward(
  owner: string,
  repo: string,
  prNumber: number
): Promise<number> {
  try {
    const pr = await octokit.pulls.get({
      owner,
      repo,
      pull_number: prNumber,
    });

    const additions = pr.data.additions;
    const deletions = pr.data.deletions;

    return deletions > additions ? deletions - additions : 0;
  } catch (error) {
    console.error("Error fetching PR data:", error);
    return 0;
  }
}
  1. edit the github comment module to also include this reward

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants