Skip to content
This repository has been archived by the owner on Sep 19, 2024. It is now read-only.

Knip CI #897

Closed
0x4007 opened this issue Nov 26, 2023 · 32 comments
Closed

Knip CI #897

0x4007 opened this issue Nov 26, 2023 · 32 comments

Comments

@0x4007
Copy link
Member

0x4007 commented Nov 26, 2023

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.

echo "::warning file=$file::This file is not in kebab-case or snake_case"


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
Copy link
Contributor

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

@0x4007
Copy link
Member Author

0x4007 commented Nov 27, 2023

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
Copy link
Contributor

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
Copy link
Member Author

0x4007 commented Nov 27, 2023

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
Copy link
Contributor

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

gitcoindev#17 (comment)

  1. 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
Copy link
Contributor

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 added a commit to gitcoindev/ubiquibot that referenced this issue Nov 28, 2023
gitcoindev added a commit to gitcoindev/ubiquibot that referenced this issue Nov 28, 2023
@gitcoindev
Copy link
Contributor

/start

Copy link

ubiquibot bot commented Nov 28, 2023

Skipping /start because it is disabled on this repo

gitcoindev added a commit to gitcoindev/ubiquibot that referenced this issue Nov 28, 2023
gitcoindev added a commit to gitcoindev/ubiquibot that referenced this issue Nov 28, 2023
@gitcoindev
Copy link
Contributor

Hi @pavlovcik I opened 0x4007#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
Copy link
Member Author

0x4007 commented Jan 31, 2024

@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
Copy link
Contributor

@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!

Copy link

ubiquibot bot commented Jan 31, 2024

! action has an uncaught error

Copy link

ubiquibot bot commented Feb 2, 2024

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

@0x4007 0x4007 closed this as completed Feb 2, 2024
Copy link

ubiquibot bot commented Feb 2, 2024

! action has an uncaught error

Copy link

ubiquibot bot commented Feb 2, 2024

+ Evaluating results. Please wait...

Copy link

ubiquibot bot commented Feb 2, 2024

! action has an uncaught error

@0x4007 0x4007 reopened this Feb 8, 2024
@0x4007 0x4007 closed this as completed Feb 8, 2024
Copy link

ubiquibot bot commented Feb 8, 2024

+ Evaluating results. Please wait...

Copy link

ubiquibot bot commented Feb 8, 2024

[ 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
Copy link
Member Author

0x4007 commented Feb 8, 2024

@gitcoindev can you try and claim this reward?

@gitcoindev
Copy link
Contributor

/query @gitcoindev

Copy link

ubiquibot bot commented Feb 8, 2024

! action has an uncaught error

@0x4007
Copy link
Member Author

0x4007 commented Feb 15, 2024

Just making sure that this has been addressed?

@gitcoindev
Copy link
Contributor

/query @gitcoindev

Copy link

ubiquibot bot commented Feb 15, 2024

Property Value
Wallet 0x7e92476D69Ff1377a8b45176b1829C4A5566653a

@gitcoindev
Copy link
Contributor

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
Copy link
Member Author

0x4007 commented Feb 15, 2024

No problem. I just voided it and will regenerate.

Copy link

ubiquibot bot commented Feb 15, 2024

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

@0x4007 0x4007 closed this as completed Feb 15, 2024
Copy link

ubiquibot bot commented Feb 15, 2024

+ Evaluating results. Please wait...

Copy link

ubiquibot bot commented Feb 15, 2024

[ 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
Copy link
Contributor

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
Copy link
Member Author

0x4007 commented Feb 15, 2024

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
Copy link
Contributor

Thank you a lot @pavlovcik !

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

No branches or pull requests

2 participants