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

Properly set the metadata value of LogReturn #41

Closed
gentlementlegen opened this issue Oct 28, 2024 · 16 comments · Fixed by #42
Closed

Properly set the metadata value of LogReturn #41

gentlementlegen opened this issue Oct 28, 2024 · 16 comments · Fixed by #42

Comments

@gentlementlegen
Copy link
Member

          Revision should point to the current version of the plugin. Within actions this is easy to get, within workers I don't think this is possible, instead we could probably use the version id?
image

Originally posted by @gentlementlegen in ubiquity-os/ubiquity-os-kernel#169 (comment)

@gentlementlegen gentlementlegen changed the title Revision should point to the current version of the plugin. Within actions this is easy to get, within workers I don't think this is possible, instead we could probably use the version id? Properly set the metadata value of LogReturn Oct 28, 2024
@imabutahersiddik

This comment was marked as spam.

Copy link

Deadline Mon, Oct 28, 6:27 PM UTC
Beneficiary 0x7245F5Cb278ea948Ab6302Bd911db00Ad4889672

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.

Copy link

Passed the deadline and no activity is detected, removing assignees: @imabutahersiddik.

@Keyrxng
Copy link

Keyrxng commented Nov 6, 2024

@gentlementlegen we gonna built it into the logger or keep it within the sdk?

@gentlementlegen
Copy link
Member Author

@Keyrxng The SDK itself uses the logger but maybe having this metadata in the logger pollutes the code base too much as it is specific to GitHub comments probably.

@Keyrxng
Copy link

Keyrxng commented Nov 8, 2024

I thought something similar, so best to keep it all within the SDK in error metadata. Can close this or transfer it or whatever as it's resolved within the SDK following the merge of gentlementlegen/ubiquity-os-kernel#1 (will PR the SDK repo now that there is one)

@gentlementlegen
Copy link
Member Author

@Keyrxng should we reopen the PR then?

@Keyrxng
Copy link

Keyrxng commented Nov 10, 2024

I'm including all of the logic for this directly into the SDK right? If so then yeah I'll PR the SDK.

Just seen foo's comment re: SDK separation complicating things. Waiting to see if the SDK moves back into the kernel or stays separate

@gentlementlegen
Copy link
Member Author

@Keyrxng Also goes for the PR you just closed I guess.

@Keyrxng
Copy link

Keyrxng commented Nov 10, 2024

@Keyrxng Also goes for the PR you just closed I guess.

Yes unfortunately, would have been ideal if you had managed to merge into your improvements PR like you suggested but it's all good I'll just re-do them.

I assume the SDK is 100% staying separate from the kernel then yeah and I should PR against it?

@gentlementlegen
Copy link
Member Author

Let's see how we do it. If the circular reference can be fixed it can remain inside the kernel and it's fine.

@gentlementlegen
Copy link
Member Author

Moving this to plugin-sdk as it handles what gets inside the metadata when postComment is called.

@gentlementlegen gentlementlegen transferred this issue from ubiquity-os/ubiquity-os-logger Dec 19, 2024
Copy link

! Error: TypeError: Cannot read properties of null (reading 'node')

Copy link

Deadline Thu, Dec 19, 3:27 PM UTC
Beneficiary 0x0fC1b909ba9265A846b82CF4CE352fc3e7EeB2ED

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.

Copy link

+ Evaluating results. Please wait...

Copy link

 [ 36 WXDAI ] 

@gentlementlegen
⚠️ Your rewards have been limited to the task price of 18 WXDAI.
Contributions Overview
ViewContributionCountReward
IssueTask118
IssueSpecification137.62
IssueComment513.014
ReviewComment10
Conversation Incentives
CommentFormattingRelevancePriorityReward
Revision should point to the current version of the plugin. With…
12.54
content:
  content:
    p:
      score: 0
      elementCount: 3
    em:
      score: 0
      elementCount: 1
    a:
      score: 5
      elementCount: 2
  result: 10
regex:
  wordCount: 45
  wordValue: 0.1
  result: 2.54
1337.62
@Keyrxng The SDK itself uses the logger but maybe having this me…
1.75
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 29
  wordValue: 0.1
  result: 1.75
0.834.2
@Keyrxng should we reopen the PR then?
0.52
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 7
  wordValue: 0.1
  result: 0.52
0.530.78
@Keyrxng Also goes for the PR you just closed I guess.
0.77
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 11
  wordValue: 0.1
  result: 0.77
0.430.924
Let's see how we do it. If the circular reference can be fixed i…
1.49
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 24
  wordValue: 0.1
  result: 1.49
134.47
Moving this to `plugin-sdk` as it handles what gets insi…
0.88
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 13
  wordValue: 0.1
  result: 0.88
132.64
Resolves #41
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 2
  wordValue: 0
  result: 0
0.130

 [ 12.402 WXDAI ] 

@Keyrxng
Contributions Overview
ViewContributionCountReward
IssueComment412.402
Conversation Incentives
CommentFormattingRelevancePriorityReward
@gentlementlegen we gonna built it into the logger or keep it wi…
0.94
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 14
  wordValue: 0.1
  result: 0.94
130.72
I thought something similar, so best to keep it all within the S…
7.54
content:
  content:
    p:
      score: 0
      elementCount: 1
    a:
      score: 5
      elementCount: 1
  result: 5
regex:
  wordCount: 45
  wordValue: 0.1
  result: 2.54
135.67
I'm including all of the logic for this directly into the SDK ri…
7.64
content:
  content:
    p:
      score: 0
      elementCount: 2
    a:
      score: 5
      elementCount: 1
  result: 5
regex:
  wordCount: 47
  wordValue: 0.1
  result: 2.64
0.834.584
Yes unfortunately, would have been ideal if you had managed to m…
2.73
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 49
  wordValue: 0.1
  result: 2.73
0.731.428

 [ 4.623 WXDAI ] 

@0x4007
Contributions Overview
ViewContributionCountReward
ReviewComment74.623
Conversation Incentives
CommentFormattingRelevancePriorityReward
```suggestionThe `postComment` function ena…
0
content:
  content: {}
  result: 0
regex:
  wordCount: 0
  wordValue: 0.1
  result: 0
0.530
This should be handled all in the SDK
0.59
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 8
  wordValue: 0.1
  result: 0.59
0.731.239
I didn't mean to post this pending comment
0.65
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 9
  wordValue: 0.1
  result: 0.65
030
Code changes look fine. Interested to see the rendered metadata…
0.94
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 14
  wordValue: 0.1
  result: 0.94
0.431.128
Looks informative but what is the second field with the value of…
0.94
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 14
  wordValue: 0.1
  result: 0.94
0.230.564
Sure we can leave it then.
0.46
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 6
  wordValue: 0.1
  result: 0.46
0.330.414
Actually the invoker field should be last for search reasons.
0.71
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 10
  wordValue: 0.1
  result: 0.71
0.631.278

 [ 18 WXDAI ] 

@whilefoo
⚠️ Your rewards have been limited to the task price of 18 WXDAI.
Contributions Overview
ViewContributionCountReward
ReviewComment925.602
Conversation Incentives
CommentFormattingRelevancePriorityReward
To get the worker name kernel needs to be updated to send URL in…
3.66
content:
  content:
    p:
      score: 0
      elementCount: 3
  result: 0
regex:
  wordCount: 69
  wordValue: 0.1
  result: 3.66
0.838.784
These two ifs could be merged
0.46
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 6
  wordValue: 0.1
  result: 0.46
0.730.966
what does `installation.account.name` refer to?
0.32
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 4
  wordValue: 0.1
  result: 0.32
0.530.48
You could just set `logMessage` to `content` in …
1.22
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 19
  wordValue: 0.1
  result: 1.22
0.632.196
why not `message instanceof LogReturn`?
0.18
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 2
  wordValue: 0.1
  result: 0.18
0.430.216
What's the point of assigning it again? I think it would look cl…
1.17
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 18
  wordValue: 0.1
  result: 1.17
0.732.457
it's fine, I just didn't know what it is
0.77
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 11
  wordValue: 0.1
  result: 0.77
0.330.693
First time I'm hearing of this. Why would `fetch` manipu…
3.61
content:
  content:
    p:
      score: 0
      elementCount: 2
    hr:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 68
  wordValue: 0.1
  result: 3.61
0.636.498
@gentlementlegen I think you missed my [comment](https://github.…
5.52
content:
  content:
    p:
      score: 0
      elementCount: 1
    a:
      score: 5
      elementCount: 1
  result: 5
regex:
  wordCount: 7
  wordValue: 0.1
  result: 0.52
0.233.312

@rndquu rndquu removed this from Development Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment