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

Empty String "" Code Review Warning Annotation #31

Closed
0x4007 opened this issue Mar 9, 2024 · 24 comments · Fixed by #62 or #66
Closed

Empty String "" Code Review Warning Annotation #31

0x4007 opened this issue Mar 9, 2024 · 24 comments · Fixed by #62 or #66

Comments

@0x4007
Copy link
Member

0x4007 commented Mar 9, 2024

Annotate them with warnings in code review to more closely scrutinize the logic that interacts with the variable.

Rationale

I see a lot of abuse for empty strings. It makes code logic sloppy and more error prone. But since there are some legitimate uses for empty strings, we will just set a warning instead of error?

Examples

https://github.com/ubiquity/audit.ubq.fi/blob/08fba85a9a592ca2bda5eaeaa2ff3b9fdd4632be/static/scripts/audit-report/audit.ts#L26C1-L27C21
  • Should be unset (which implies undefined) instead of passing an empty string as a credential or URL. Then the code should check if the value is truthy before expecting authentication.
let REPOSITORY_URL;
let GITHUB_PAT;
https://github.com/ubiquity/ubiquibot-kernel/blame/c553d5a7866c1ef76d4f589d8095c8bb751cbd49/src/github/types/config.ts#L29
  • Should be value.ref ?? "@" + value.ref as I'm assuming the developer never expected the "" value to be returned there.
https://github.com/ubiquity/pay.ubq.fi/pull/189/files#r1518549017
  • This shouldn't have been here at all. It's not a valid RPC endpoint.

From codebase search: https://github.com/search?q=org%3Aubiquity+%5C%22%5C%22+language%3ATypeScript&type=code
@Keyrxng
Copy link
Member

Keyrxng commented Mar 25, 2024

/start

Copy link

ubiquibot bot commented Mar 25, 2024

DeadlineMon, Mar 25, 8:54 AM UTC
Registered Wallet 0xAe5D1F192013db889b1e2115A370aB133f359765
Tips:
  • 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.

@Keyrxng
Copy link
Member

Keyrxng commented Jun 4, 2024

/stop if it is still an open task I will pick it back up. Feel free to ref my PR if anyone wants to take this

@ubiquibot ubiquibot bot unassigned Keyrxng Jun 4, 2024
Copy link

ubiquibot bot commented Jun 4, 2024

+ You have been unassigned from the task

@Sadaf-A
Copy link

Sadaf-A commented Jul 5, 2024

/start

Copy link

ubiquibot bot commented Jul 5, 2024

Warning! This task was created over 118 days ago. Please confirm that this issue specification is accurate before starting.
DeadlineFri, Jul 5, 11:19 PM UTC
Registered Wallet 0x0BEd00438D57d07E3667b85Fa8EB86Af147C7025
Tips:
  • 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.

Copy link

ubiquibot bot commented Aug 16, 2024

# These linked pull requests are closed:  <a href="https://github.com/ubiquity/ts-template/pull/36">#36</a> 

@jordan-ae
Copy link
Contributor

/start

Copy link

ubiquity-os bot commented Sep 4, 2024

Warning! This task was created over 179 days ago. Please confirm that this issue specification is accurate before starting.
DeadlineWed, Sep 4, 6:21 PM UTC
Registered Wallet 0x2F05fD58023B0a95d1866aa0A3b672cEf05945c5
Tips:
  • 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.

Copy link

ubiquity-os bot commented Sep 10, 2024

@jordan-ae, this task has been idle for a while. Please provide an update.

@jordan-ae
Copy link
Contributor

/stop

Copy link

ubiquity-os bot commented Sep 12, 2024

# These linked pull requests are closed:  https://github.com/ubiquity/ts-template/pull/57 

Copy link

ubiquity-os bot commented Oct 8, 2024

! You have reached your max task limit. Please close out some tasks before assigning new ones.

@0x4007 0x4007 closed this as completed in #62 Oct 8, 2024
Copy link

ubiquity-os bot commented Oct 8, 2024

! Failed to run comment evaluation. TypeError: Cannot read properties of undefined (reading 'wordValue')

@0x4007 0x4007 reopened this Oct 8, 2024
This was referenced Oct 8, 2024
Copy link

ubiquity-os bot commented Oct 8, 2024

! You have reached your max task limit. Please close out some tasks before assigning new ones.

Copy link

ubiquity-os bot commented Oct 9, 2024

! You have reached your max task limit. Please close out some tasks before assigning new ones.

1 similar comment
Copy link

ubiquity-os bot commented Oct 9, 2024

! You have reached your max task limit. Please close out some tasks before assigning new ones.

@0x4007 0x4007 closed this as completed in #66 Oct 9, 2024
@0x4007 0x4007 self-assigned this Oct 9, 2024
Copy link

ubiquity-os bot commented Oct 9, 2024

! Failed to run comment evaluation. TypeError: Cannot read properties of undefined (reading 'wordValue')

Copy link

ubiquity-os bot commented Oct 9, 2024

@0x4007 the deadline is at Wed, Oct 9, 2:18 AM UTC

@0x4007 0x4007 reopened this Oct 9, 2024
@0x4007 0x4007 closed this as completed Oct 9, 2024
Copy link

ubiquity-os bot commented Oct 9, 2024

! Failed to run comment evaluation. TypeError: Cannot read properties of undefined (reading 'wordValue')

@0x4007
Copy link
Member Author

0x4007 commented Oct 9, 2024

! Failed to run comment evaluation. TypeError: Cannot read properties of undefined (reading 'wordValue')

@gentlementlegen can you fix today and leverage Whilefoo environment config switcher

Copy link

ubiquity-os bot commented Oct 18, 2024

 [ 156.074 WXDAI ] 

@0x4007
Contributions Overview
ViewContributionCountReward
IssueTask1100
IssueSpecification155.92
IssueComment10.154
ReviewComment50
Conversation Incentives
CommentFormattingRelevanceReward
Annotate them with warnings in code review to more closely scrut…
18.64
content:
  content:
    p:
      score: 0
      elementCount: 2
    h3:
      score: 1
      elementCount: 2
    h6:
      score: 1
      elementCount: 4
    ul:
      score: 1
      elementCount: 3
    li:
      score: 0.5
      elementCount: 3
    hr:
      score: 0
      elementCount: 1
  result: 10.5
regex:
  wordCount: 177
  wordValue: 0.1
  result: 8.14
155.92
@gentlementlegen can you fix today and leverage Whilefoo enviro…
0.77
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 11
  wordValue: 0.1
  result: 0.77
0.20.154
Resolves https://github.com/ubiquity/ts-template/issues/31Curr…
0
content:
  content:
    p:
      score: 0
      elementCount: 3
  result: 0
regex:
  wordCount: 67
  wordValue: 0
  result: 0
0.80
I got annotations working on the files view but I can't get the …
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 19
  wordValue: 0
  result: 0
0.40
Currently matches any instance of `""` but with regex ca…
0
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 43
  wordValue: 0
  result: 0
0.70
It's a warning not an error. This expresses that the reviewers s…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 59
  wordValue: 0
  result: 0
0.60
Also I just realized that it doesn't display annotations on GitH…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 26
  wordValue: 0
  result: 0
0.30

 [ 1.192 WXDAI ] 

@gentlementlegen
Contributions Overview
ViewContributionCountReward
ReviewComment11.192
Conversation Incentives
CommentFormattingRelevanceReward
@0x4007 It seems you merged this with empty strings: https://git…
1.49
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 24
  wordValue: 0.1
  result: 1.49
0.81.192

@ubiquity ubiquity deleted a comment from ubiquity-os bot Oct 18, 2024
Copy link

ubiquity-os bot commented Oct 28, 2024

 [ 156.151 WXDAI ] 

@0x4007
Contributions Overview
ViewContributionCountReward
IssueTask1100
IssueSpecification155.92
IssueComment10.231
ReviewComment50
Conversation Incentives
CommentFormattingRelevanceReward
Annotate them with warnings in code review to more closely scrut…
18.64
content:
  content:
    p:
      score: 0
      elementCount: 2
    h3:
      score: 1
      elementCount: 2
    h6:
      score: 1
      elementCount: 4
    ul:
      score: 1
      elementCount: 3
    li:
      score: 0.5
      elementCount: 3
    hr:
      score: 0
      elementCount: 1
  result: 10.5
regex:
  wordCount: 177
  wordValue: 0.1
  result: 8.14
155.92
@gentlementlegen can you fix today and leverage Whilefoo enviro…
0.77
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 11
  wordValue: 0.1
  result: 0.77
0.30.231
Resolves https://github.com/ubiquity/ts-template/issues/31Curr…
0
content:
  content:
    p:
      score: 0
      elementCount: 3
  result: 0
regex:
  wordCount: 67
  wordValue: 0
  result: 0
0.80
I got annotations working on the files view but I can't get the …
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 19
  wordValue: 0
  result: 0
0.50
Currently matches any instance of `""` but with regex ca…
0
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 43
  wordValue: 0
  result: 0
0.70
It's a warning not an error. This expresses that the reviewers s…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 59
  wordValue: 0
  result: 0
0.60
Also I just realized that it doesn't display annotations on GitH…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 26
  wordValue: 0
  result: 0
0.40

 [ 1.192 WXDAI ] 

@gentlementlegen
Contributions Overview
ViewContributionCountReward
ReviewComment11.192
Conversation Incentives
CommentFormattingRelevanceReward
@0x4007 It seems you merged this with empty strings: https://git…
1.49
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 24
  wordValue: 0.1
  result: 1.49
0.81.192

@gentlementlegen
Copy link
Member

@0x4007 I should have tested this, but it will always crash if a user opens a pull request because it uses variables from secrets that are not accessible, I will fix that.

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