ubiquity / ubiquibot

Putting the 'A' in 'DAO'
https://github.com/marketplace/ubiquibot
MIT License
16 stars 59 forks source link

Knip CI #897

Closed 0x4007 closed 4 months ago

0x4007 commented 7 months ago

Use Knip in a GitHub Action and annotate all of the unused code inside of the code view in pull requests.

on: [push, pull_request]

You can see an example of how to annotate in our kebab-case.yml however the annotations should be specific to the line that must be removed. https://github.com/ubiquity/ubiquibot/blob/c015fc791afdec32856d1c30855273b05b587a86/.github/workflows/kebab-case.yml#L42


Knip is great to keep code bloat down, but I wasn't sure how to have it automatically run (I don't think it makes sense to as a commit hook, for example.)

gitcoindev commented 7 months ago

I almost have it integrated, need still to execute a few tests...

0x4007 commented 7 months ago

So I realized only today that I have mixed feelings on this.

Knip is fantastic however there are some identifiers which are used in runtime which I'm not sure how to annotate in knip.

For example, issue state enum has three values: opened, closed, reopened (if I recall correctly)

We only directly reference opened in the code so knip is satisfied. The other two it complains about.

When I removed the other two, ajv was throwing a validation error on the payload because they were missing.

Any ideas @gitcoindev

gitcoindev commented 7 months ago

hi @pavlovcik I am still experimenting on GitHub actions, need about 1-2 hrs to open a pr.

I discovered that Knip has a quite flexible configuration if it provides false positives: https://knip.dev/guides/handling-issues/ I am currently using ignored config.ts for testing. It is also possible to get more fine grained ignores, I am currently using a half-baked cfg:

import type { KnipConfig } from "knip";

const config: KnipConfig = {
  entry: ["src/index.ts"],
  project: ["src/**/*.ts"],
  ignore: ["src/types/config.ts"],
  ignoreExportsUsedInFile: true,
  ignoreDependencies: [],
};

export default config;

So in all we should be able to fine tune it to get the desired output, also check or skip the required GitHub action enum values. To be honest today is the first time I have used it but it looks promising. It is also actively developed, v3 was released a month ago.

0x4007 commented 7 months ago

By the way. For the sake of saving us time with merge conflicts, would you mind working off of my refactor/general branch off my fork?

@wannacfuture is tidying up some final runtime errors and then we should be good to merge and deploy!

gitcoindev commented 7 months ago

By the way. For the sake of saving us time with merge conflicts, would you mind working off of my refactor/general branch off my fork?

@wannacfuture is tidying up some final runtime errors and then we should be good to merge and deploy!

Yes, sure. Btw. great news!

So far I have a still dirty (but working) version on my fork. It gives

1) a comment

https://github.com/gitcoindev/ubiquibot/pull/17#issuecomment-1828200946

2) unused class members and class members annotations directly in the source comments

https://github.com/gitcoindev/ubiquibot/pull/17/files#diff-17b7f13e2351a5040cebff355503a16fdc9dfbd30045c0d12b26e4c1f57f1c4f

I analysed the extended JSON output from Knip and found out that it also gives pointers to unused exports so this could also potentially be injected directly as comments but they are not parsed in the default GitHub action, yet. I forked the GitHub action and will try to extend it.

An example for enums:

 {u'binaries': [],
  u'classMembers': {},
  u'dependencies': [],
  u'devDependencies': [],
  u'duplicates': [],
  u'enumMembers': {u'StateReason': [{u'col': 3,
                                     u'line': 47,
                                     u'name': u'NOT_PLANNED',
                                     u'pos': 1092},
                                    {u'col': 3,
                                     u'line': 48,
                                     u'name': u'REOPENED',
                                     u'pos': 1123}],
                   u'UserType': [{u'col': 3,
                                  u'line': 36,
                                  u'name': u'Organization',
                                  u'pos': 923}]},
  u'exports': [],
  u'file': u'src/types/payload.ts',
  u'files': False,
  u'optionalPeerDependencies': [],
  u'owners': [u'@0xcodercrane'],
  u'types': [],
  u'unlisted': [],
  u'unresolved': []},

and also exports:

{u'binaries': [],
  u'classMembers': {},
  u'dependencies': [],
  u'devDependencies': [],
  u'duplicates': [],
  u'enumMembers': {},
  u'exports': [{u'col': 14,
                u'line': 334,
                u'name': u'unusedAction',
                u'pos': 11973},
               {u'col': 14,
                u'line': 339,
                u'name': u'unusedAction2',
                u'pos': 12075},
               {u'col': 14,
                u'line': 344,
                u'name': u'unusedAction3',
                u'pos': 12179}],
  u'file': u'src/handlers/payout/action.ts',
  u'files': False,
  u'optionalPeerDependencies': [],
  u'owners': [u'@0xcodercrane'],
  u'types': [],
  u'unlisted': [],

I will try to get those working as well and submit a PR based on refactor/general.

gitcoindev commented 7 months ago

All right I have this working. I enabled also unused exports annotations directly in the sources, which was not available before. I have it running on a GitHub action fork but asked the author if he wants to pull it in. This is pretty cool, I searched GitHub and I am pretty sure we are the first ones to integrate and extend it.

Screenshot from 2023-11-28 12-23-08

I will now prepare a PR based on refactor/general and provide QA.

The only thing remaining will be to configure Knip to give us the desired output and ignore what should be ignored.

gitcoindev commented 7 months ago

/start

ubiquibot[bot] commented 7 months ago

Skipping /start because it is disabled on this repo

gitcoindev commented 7 months ago

Hi @pavlovcik I opened https://github.com/pavlovcik/ubiquibot/pull/57 this pull request targets directly your refactor/general branch. I am fine with leaving this pull request in limbo until refactor/general branch is merged to development. I will then redirect the pull request to development and execute start command after contributions are enabled again.

0x4007 commented 5 months ago

@gitcoindev I just realized that you should have received credit for this task.

Once the payout flow is working again let's address this.

Thanks for your contribution!

gitcoindev commented 5 months ago

@gitcoindev I just realized that you should have received credit for this task.

Once the payout flow is working again let's address this.

Thanks for your contribution!

Thank you, too, for addressing this!

ubiquibot[bot] commented 5 months ago
! action has an uncaught error
ubiquibot[bot] commented 5 months ago

@gitcoindev the deadline is at 2024-02-02T14:28:52.758Z

ubiquibot[bot] commented 5 months ago
! action has an uncaught error
ubiquibot[bot] commented 5 months ago
+ Evaluating results. Please wait...
ubiquibot[bot] commented 5 months ago
! action has an uncaught error
ubiquibot[bot] commented 5 months ago
+ Evaluating results. Please wait...
ubiquibot[bot] commented 5 months ago

[ 56.8 WXDAI ]

@pavlovcik
Contributions Overview
ViewContributionCountReward
IssueSpecification123.4
IssueComment333.4
Conversation Incentives
CommentFormattingRelevanceReward
Use [Knip](https://knip.dev/) in a GitHub Action and annotate al...
23.4
a:
  count: 1
  score: "1"
  words: 1
code:
  count: 2
  score: "2"
  words: 3
hr:
  count: 1
  score: "1"
  words: 0
123.4
So I realized only today that I have mixed feelings on this. K...
180.7918
By the way. For the sake of saving us time with merge conflicts,...
9.8
code:
  count: 1
  score: "1"
  words: 2
0.79.8
@gitcoindev I just realized that you should have received credit...
5.60.8555.6

[ 296.5 WXDAI ]

@gitcoindev
Contributions Overview
ViewContributionCountReward
IssueTask1.00225
IssueComment60
IssueComment671.5
Conversation Incentives
CommentFormattingRelevanceReward
I _almost_ have it integrated, need still to execute a few tests...
-0.65-
hi @pavlovcik I am still experimenting on GitHub actions, need a...
-
code:
  count: 1
  score: "0"
  words: 0
0.77-
> By the way. For the sake of saving us time with merge conflict...
-
li:
  count: 2
  score: "0"
  words: 14
code:
  count: 4
  score: "0"
  words: 4
0.64-
All right I have this working. I enabled also unused exports ann...
-
code:
  count: 1
  score: "0"
  words: 2
0.76-
Hi @pavlovcik I opened https://github.com/pavlovcik/ubiquibot/pu...
-
code:
  count: 4
  score: "0"
  words: 6
0.755-
> @gitcoindev I just realized that you should have received cred...
-0.85-
I _almost_ have it integrated, need still to execute a few tests...
1.10.651.1
hi @pavlovcik I am still experimenting on GitHub actions, need a...
15.4
code:
  count: 1
  score: "1"
  words: 0
0.7715.4
> By the way. For the sake of saving us time with merge conflict...
34.1
li:
  count: 2
  score: "2"
  words: 14
code:
  count: 4
  score: "4"
  words: 4
0.6434.1
All right I have this working. I enabled also unused exports ann...
10.7
code:
  count: 1
  score: "1"
  words: 2
0.7610.7
Hi @pavlovcik I opened https://github.com/pavlovcik/ubiquibot/pu...
9.6
code:
  count: 4
  score: "4"
  words: 6
0.7559.6
> @gitcoindev I just realized that you should have received cred...
0.60.850.6
0x4007 commented 5 months ago

@gitcoindev can you try and claim this reward?

gitcoindev commented 5 months ago

/query @gitcoindev

ubiquibot[bot] commented 5 months ago
! action has an uncaught error
0x4007 commented 4 months ago

Just making sure that this has been addressed?

gitcoindev commented 4 months ago

/query @gitcoindev

ubiquibot[bot] commented 4 months ago
Property Value
Wallet 0x7e92476D69Ff1377a8b45176b1829C4A5566653a
gitcoindev commented 4 months ago

Hi @pavlovcik , wallet address is set correctly now, but the old permit is pointing to a wrong address. The permit would have to be regenerated.

0x4007 commented 4 months ago

No problem. I just voided it and will regenerate.

ubiquibot[bot] commented 4 months ago

@gitcoindev the deadline is at 2024-02-15T18:09:26.405Z

ubiquibot[bot] commented 4 months ago
+ Evaluating results. Please wait...
ubiquibot[bot] commented 4 months ago

[ 61.8 WXDAI ]

@pavlovcik
Contributions Overview
ViewContributionCountReward
IssueSpecification123.4
IssueComment638.4
Conversation Incentives
CommentFormattingRelevanceReward
Use [Knip](https://knip.dev/) in a GitHub Action and annotate al...
23.4
a:
  count: 1
  score: "1"
  words: 1
code:
  count: 2
  score: "2"
  words: 3
hr:
  count: 1
  score: "1"
  words: 0
123.4
So I realized only today that I have mixed feelings on this. K...
180.6318
By the way. For the sake of saving us time with merge conflicts,...
9.8
code:
  count: 1
  score: "1"
  words: 2
0.7359.8
@gitcoindev I just realized that you should have received credit...
5.60.835.6
@gitcoindev can you try and claim this reward?...
1.60.7951.6
Just making sure that this has been addressed?...
1.60.7551.6
No problem. I just voided it and will regenerate....
1.80.761.8

[ 299 WXDAI ]

@gitcoindev
Contributions Overview
ViewContributionCountReward
IssueTask1.00225
IssueComment70
IssueComment774
Conversation Incentives
CommentFormattingRelevanceReward
I _almost_ have it integrated, need still to execute a few tests...
-0.555-
hi @pavlovcik I am still experimenting on GitHub actions, need a...
-
code:
  count: 1
  score: "0"
  words: 0
0.78-
> By the way. For the sake of saving us time with merge conflict...
-
li:
  count: 2
  score: "0"
  words: 14
code:
  count: 4
  score: "0"
  words: 4
0.72-
All right I have this working. I enabled also unused exports ann...
-
code:
  count: 1
  score: "0"
  words: 2
0.71-
Hi @pavlovcik I opened https://github.com/pavlovcik/ubiquibot/pu...
-
code:
  count: 4
  score: "0"
  words: 6
0.72-
> @gitcoindev I just realized that you should have received cred...
-0.76-
Hi @pavlovcik , wallet address is set correctly now, but the old...
-0.74-
I _almost_ have it integrated, need still to execute a few tests...
1.10.5551.1
hi @pavlovcik I am still experimenting on GitHub actions, need a...
15.4
code:
  count: 1
  score: "1"
  words: 0
0.7815.4
> By the way. For the sake of saving us time with merge conflict...
34.1
li:
  count: 2
  score: "2"
  words: 14
code:
  count: 4
  score: "4"
  words: 4
0.7234.1
All right I have this working. I enabled also unused exports ann...
10.7
code:
  count: 1
  score: "1"
  words: 2
0.7110.7
Hi @pavlovcik I opened https://github.com/pavlovcik/ubiquibot/pu...
9.6
code:
  count: 4
  score: "4"
  words: 6
0.729.6
> @gitcoindev I just realized that you should have received cred...
0.60.760.6
Hi @pavlovcik , wallet address is set correctly now, but the old...
2.50.742.5
gitcoindev commented 4 months ago

Interesting... it throws 'THIS REWARD HAS ALREADY BEEN CLAIMED OR INVALIDATED.' error (I am sorry for caps it was copy pasted -). Could you please check your permit?

0x4007 commented 4 months ago

Interesting... it throws 'THIS REWARD HAS ALREADY BEEN CLAIMED OR INVALIDATED.' error (I am sorry for caps it was copy pasted -). Could you please check your permit?

It lets me claim mine. I realize that the nonce encodes the issue id and your GitHub user id. The wrong wallet was associated to your user ID and I invalidated it I think (I don't remember.)

I'll just send you a manual payment then. https://gnosisscan.io/tx/0x1a576044cb22bff3b54311f6751cfc1ca043bc30c57df8b6491761a15a57cbe1

gitcoindev commented 4 months ago

Thank you a lot @pavlovcik !