-
Notifications
You must be signed in to change notification settings - Fork 20
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
Collaborator Gating Based On Label #62
Collaborator Gating Based On Label #62
Comments
@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 |
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 |
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 |
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. |
|
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 ubiquity-os-marketplace/daemon-pricing#18 to be completed in order to have the label updates working properly. |
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. |
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. |
1 similar comment
@gentlementlegen, this task has been idle for a while. Please provide an update. |
Still waiting on ubiquity-os-marketplace/daemon-pricing#18 to be merged in. |
ubiquity-os-marketplace/daemon-pricing#42 (comment) Since ubiquity-os-marketplace/daemon-pricing#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
It really is as easy as reading from the config when the user calls - 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 "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. |
@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 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. |
Didn't notice this my mistake |
|
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Task | 1 | 200 |
Issue | Comment | 15 | 0 |
Review | Comment | 2 | 0 |
Conversation Incentives
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 |
[ 24.6465 WXDAI ]
@0x4007
Contributions Overview
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Specification | 1 | 15.48 |
Issue | Comment | 11 | 8.5005 |
Review | Comment | 2 | 0.666 |
Conversation Incentives
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. | 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 |
[ 4.42 WXDAI ]
@Keyrxng
Contributions Overview
View | Contribution | Count | Reward |
---|---|---|---|
Issue | Comment | 4 | 4.42 |
Conversation Incentives
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.
The text was updated successfully, but these errors were encountered: