Closed 0x4007 closed 1 week ago
@gentlementlegen I think this could save us from a lot of unnecessary blockers. Perhaps you can prioritize it?
@gentlementlegen the deadline is at Tue, Oct 1, 5:41 PM UTC
@0x4007 Should we add a tag on the task or should it just be taken from the configuration?
Just from the configuration. I don't think extra labels is a good idea.
@0x4007 Any message to display like "This issue is for collaborators only?"
In the future I think its worthwhile to assume the correct answer for these small details that are fast to change to reduce delay. This message seems fine to start!
@0x4007 I just realized this was created under the configuration of assistive-pricing
, so I think we should create a new label on the issue for this so command-start-stop
can check it and know if the task is restricted or not.
I don't think extra labels is a good idea.
To me it seems like a lazy answer to keep adding labels for every feature. We already went through this with v1 of bot and the issues all looked like a mess.
First idea then would be to add a description inside the label saying "Collaborator only" so that command-start-stop
can check the restrictions on the labels, without having to add new ones.
We could even embed metadata in the label description, although for this purpose maybe via the config here is simple enough.
I don't think it makes sense to implement a new approach. Just read from config like we always do because it works fine. We can embed metadata in the description if we have more complex needs.
The configuration you linked is not for this plugin, plugins are not aware of the configuration of the others which is what I am trying to solve here.
Currently command-start-stop
reads the labels that were set from assistive-pricing
which is the only way they convey information to each other so far. Which is why I first proposed to have another tag. But I think adding the restrictions within the tag themselves allows for easy parsing and does not add much more logic, and no new tags. Otherwise I am not sure how to approach this.
Then you have to sync. It's more complicated than just reading from config every time I think
What do you mean by "to sync"?
Any changes to the config have to sync all the descriptions every time with your proposal. It's more complex.
assistive-pricing
already syncs all the labels whenever there is a change in pricing / label added / labels removed / labels edited so I don't think more logic is needed.
Then let's embed in the description if that means less new code.
Given that we don't do that anywhere else, and that we do something more similar to my proposal, I am highly skeptical that it is the simplest way.
Will depend on https://github.com/ubiquity-os-marketplace/daemon-pricing/pull/18 to be completed in order to have the label updates working properly.
So through the UI the labels would look like this
which I think is helpful when creating the tasks.
Perhaps we can use emoji so that it stands out like ⚠️
Not sure if GitHub supports that, will have a look.
To clarify I mean in the description. ⚠️ contributor only.
Yes got it. It seems to be supported:
Price shoudn't have that
@0x4007 Yes it was for testing, I didn't remove that tag, pricing won't have them.
! All linked pull requests must be closed to generate rewards.
@gentlementlegen, this task has been idle for a while. Please provide an update.
@gentlementlegen, this task has been idle for a while. Please provide an update.
Still waiting on https://github.com/ubiquity-os-marketplace/daemon-pricing/pull/18 to be merged in.
https://github.com/ubiquity-os-marketplace/daemon-pricing/pull/42#issuecomment-2431077191
Since https://github.com/ubiquity-os-marketplace/daemon-pricing/pull/18 is not actually blocking this and after looking over things I'd like offer a super simple ultra low code suggestion that can be merged in under an hour
To me it seems like a lazy answer to keep adding labels for every feature. We already went through this with v1 of bot and the issues all looked like a mess.
Then you have to sync. It's more complicated than just reading from config every time I think
It really is as easy as reading from the config when the user calls /start
, then if the task labels are lower than the new config thresholds they can start it.
- uses:
- plugin: http://localhost:4000
runsOn: ["issue_comment.created", "issues.assigned", "issues.unassigned", "pull_request.opened", "pull_request.edited"]
with:
reviewDelayTolerance: "1 Day"
taskStaleTimeoutDuration: "30 Days"
maxConcurrentTasks:
member: 2
contributor: 5
startRequiresWallet: false
rolesWithReviewAuthority: ["OWNER", "ADMIN", "MEMBER", "CONTRIBUTOR"]
collaboratorOnlyThresholds:{
time: 1 week,
priority: 4 urgent
}
The benefits of using assistive-pricing
I guess is being able to update label descriptions but that was never a concern before, as a comment on the task is usually the chosen approach to user feedback as opposed to bury inside label info which a contributor may not see unless they open up the label selection dropdown, and ultimately we'd need to serve feedback from /start
anyway.
"time" and "priority" prefixes may be changed by partners I think but not us so can deal with that later if that's an issue reading this approach.
example, I can't set labels in this org so I cannot see all of the available labels, meaning this description is hidden to contributors, so if they use /start
we'll need to provide feedback anyway.
@Keyrxng Thanks for the idea! However this is all already implemented there so I could change this after we merge https://github.com/ubiquity-os-marketplace/daemon-pricing/pull/42
@Keyrxng Thanks for the idea! However this is all already implemented there so I could change this after we merge ubiquity-os-marketplace/daemon-pricing#42
I saw that PR was merged yeah, although if the descriptions are only visible to people with access it's more of an internal quality of life having label descriptions since only team can see them.
Since they are not dependant on each other the change could be made in start-stop
first and not be held back by me or any other bugs or whatever, if the plan is to change it later, may as well do it first as there is nothing holding that back.
Sorry, just trying to provide value/solutions vs problems and delays considering how things are rn, hope you understand.
Actually if you pass your cursor over the tags you can see the "collaborator only". And either way when users try to assign they would get a message.
Sure but we need to configuration change feature anyway.
Actually if you pass your cursor over the tags you can see the "collaborator only".
Didn't notice this my mistake
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Task | 1 | 200 |
Issue | Comment | 15 | 0 |
Review | Comment | 2 | 0 |
Comment | Formatting | Relevance | Reward |
---|---|---|---|
@0x4007 Should we add a tag on the task or should it just be tak… | 0content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 18 wordValue: 0 result: 0 | 0.85 | 0 |
@0x4007 Any message to display like "This issue is for collabora… | 0content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 12 wordValue: 0 result: 0 | 0.9 | 0 |
@0x4007 I just realized this was created under the configuration… | 0content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 38 wordValue: 0 result: 0 | 0.88 | 0 |
First idea then would be to add a description inside the label s… | 0content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 30 wordValue: 0 result: 0 | 0.87 | 0 |
The configuration you linked is not for this plugin, plugins are… | 0content: content: p: score: 0 elementCount: 2 result: 0 regex: wordCount: 93 wordValue: 0 result: 0 | 0.92 | 0 |
What do you mean by "to sync"? | 0content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 7 wordValue: 0 result: 0 | 0.8 | 0 |
`assistive-pricing` already syncs all the labels wheneve… | 0content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 27 wordValue: 0 result: 0 | 0.91 | 0 |
Will depend on https://github.com/ubiquity-os-marketplace/daemon… | 0content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 25 wordValue: 0 result: 0 | 0.65 | 0 |
So through the UI the labels would look like this![image](http… | 5content: content: p: score: 0 elementCount: 3 img: score: 5 elementCount: 1 result: 5 regex: wordCount: 19 wordValue: 0 result: 0 | 0.6 | 0 |
Not sure if GitHub supports that, will have a look. | 0content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 10 wordValue: 0 result: 0 | 0.6 | 0 |
Yes got it. It seems to be supported:![image](https://github.c… | 5content: content: p: score: 0 elementCount: 2 img: score: 5 elementCount: 1 result: 5 regex: wordCount: 8 wordValue: 0 result: 0 | 0.55 | 0 |
@0x4007 Yes it was for testing, I didn't remove that tag, pricin… | 0content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 17 wordValue: 0 result: 0 | 0.82 | 0 |
Still waiting on https://github.com/ubiquity-os-marketplace/daem… | 0content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 17 wordValue: 0 result: 0 | 0.7 | 0 |
@Keyrxng Thanks for the idea! However this is all already implem… | 5content: content: p: score: 0 elementCount: 1 a: score: 5 elementCount: 1 result: 5 regex: wordCount: 30 wordValue: 0 result: 0 | 0.75 | 0 |
Actually if you pass your cursor over the tags you can see the "… | 0content: content: p: score: 0 elementCount: 2 result: 0 regex: wordCount: 37 wordValue: 0 result: 0 | 0.78 | 0 |
Resolves #62 | 0content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 2 wordValue: 0 result: 0 | 0.5 | 0 |
@0x4007 should not happenhttps://github.com/ubiquity-os-market… | 0content: content: p: score: 0 elementCount: 2 result: 0 regex: wordCount: 22 wordValue: 0 result: 0 | 0.2 | 0 |
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Specification | 1 | 15.48 |
Issue | Comment | 11 | 8.5005 |
Review | Comment | 2 | 0.666 |
Comment | Formatting | Relevance | Reward |
---|---|---|---|
- Since we still are ways off to implementing XP, we should impl… | 5.16content: content: ul: score: 0 elementCount: 1 li: score: 0.5 elementCount: 3 p: score: 0 elementCount: 4 result: 1.5 regex: wordCount: 69 wordValue: 0.1 result: 3.66 | 1 | 15.48 |
@gentlementlegen I think this could save us from a lot of unnece… | 1.17content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 18 wordValue: 0.1 result: 1.17 | 0.85 | 0.9945 |
Just from the configuration. I don't think extra labels is a goo… | 0.94content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 14 wordValue: 0.1 result: 0.94 | 0.75 | 0.705 |
In the future I think its worthwhile to assume the correct answe… | 1.8content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 30 wordValue: 0.1 result: 1.8 | 0.7 | 1.26 |
To me it seems like a lazy answer to keep adding labels for ever… | 1.9content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 32 wordValue: 0.1 result: 1.9 | 0.65 | 1.235 |
I don't think it makes sense to implement a new approach. Just r… | 2.15content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 37 wordValue: 0.1 result: 2.15 | 0.6 | 1.29 |
Then you have to sync. It's more complicated than just reading f… | 1.17content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 18 wordValue: 0.1 result: 1.17 | 0.55 | 0.6435 |
Any changes to the config have to sync all the descriptions ever… | 1.28content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 20 wordValue: 0.1 result: 1.28 | 0.5 | 0.64 |
Then let's embed in the description if that means less new code.… | 2.4content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 42 wordValue: 0.1 result: 2.4 | 0.45 | 1.08 |
Perhaps we can use emoji so that it stands out like ⚠️ | 0.77content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 11 wordValue: 0.1 result: 0.77 | 0.4 | 0.308 |
To clarify I mean in the description. ⚠️ contributor only. | 0.65content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 9 wordValue: 0.1 result: 0.65 | 0.35 | 0.2275 |
Price shoudn't have that | 0.39content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 5 wordValue: 0.1 result: 0.39 | 0.3 | 0.117 |
```suggestionthrow logger.error("Only co… | 0content: content: {} result: 0 regex: wordCount: 0 wordValue: 0.1 result: 0 | 0.9 | 0 |
Just make sure that you don't change the Price label description… | 1.11content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 17 wordValue: 0.1 result: 1.11 | 0.6 | 0.666 |
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Comment | 4 | 4.42 |
Comment | Formatting | Relevance | Reward |
---|---|---|---|
https://github.com/ubiquity-os-marketplace/daemon-pricing/pull/4… | 8.06content: content: p: score: 0 elementCount: 5 result: 0 regex: wordCount: 175 wordValue: 0.1 result: 8.06 | 0.85 | 1.71775 |
example, I can't set labels in this org so I cannot see all of t… | 7.1content: content: p: score: 0 elementCount: 2 img: score: 5 elementCount: 1 result: 5 regex: wordCount: 36 wordValue: 0.1 result: 2.1 | 0.8 | 1.675 |
I saw that PR was merged yeah, although if the descriptions are … | 5.18content: content: p: score: 0 elementCount: 3 result: 0 regex: wordCount: 104 wordValue: 0.1 result: 5.18 | 0.75 | 0.97625 |
Didn't notice this my mistake | 0.46content: content: p: score: 0 elementCount: 1 result: 0 regex: wordCount: 6 wordValue: 0.1 result: 0.46 | 0.4 | 0.051 |
We could even embed metadata in the label description, although for this purpose maybe via the config here is simple enough.