Comments (31)
Just realized that the identity is stored on the relay chain and the fellowship collectives on the collectives parachain. So, it requires connecting to two chains. However, smoldot should do this internally already to find out what is the best block etc, but I'm not sure if both connected chains can be "used".
I think a map from github name to on-chain address is better managed in this repo.
Updating your identity on-chain unfortunately is not super easy, and will remove your "verified" status unless you pay an identity registrar to re-verify your account.
Some things can be done to make this better, for example giving the fellowship itself an identity registrar spot, which makes sense to me, but I still feel that maintaining a simple lookup table makes more sense here than on-chain.
from runtimes.
i mean would you for example require only people since dan #3 to be able to approve?
Another point to be raised with this is that disallowing lower ranks to review proposals would hinder even more their ability to rise to upper ranks. In my opinion, all members should be able to participate in GitHub proposal reviews with approval rights, even if that approval has a lower weight compared to higher ranks.
from runtimes.
People can always comment/review any PR without any permissions. It is just that those won't be considered for merge. I have been reviewing and commenting Substrate PR for years, and that's how I participate and rank up myself. I did not need to ask anyone for permissions to start.
from runtimes.
I just want to keep it simple. I hear your request and sounds reasonable but at the same time do you really think it is a must have feature for the initial release? Let's get something out first and then consider improving it. Not the other way around.
I agree, but it's important to have these discussions even if not implemented immediately, which is why I commented on it now.
from runtimes.
For merging we should then have some kind of github action bot that monitors comments on prs. When someone writes bot merge
and all CI checks (also the approval check outlined above) are green, the bot should merge the pr. The bot should maybe do some check that the person writing bot merge
is a fellowship member, just to prevent spam or stuff getting merged randomly. (It will still require an approval, but yeah)
from runtimes.
Just realized that the identity is stored on the relay chain and the fellowship collectives on the collectives parachain. So, it requires connecting to two chains. However, smoldot should do this internally already to find out what is the best block etc, but I'm not sure if both connected chains can be "used".
from runtimes.
The list of fellowship members is maintained on chain, so it would introduce extra overhead to maintain the list on github as well. Like having teams for each rank and then having some CI check that succeeds when "the voting" has passed.
Feels like we could instead do exactly that, and either reuse PRCR, or even use CODEOWNERS.
Synchronising dans from chain to GitHub teams would also bring an ability to grant other types of GH-permissions (e.g. admin, force-push) to fellows of specific dan.
Isn't that also a problem that needs solving?
from runtimes.
map from a github name to the fellowship rank to do "the voting".
@bkchr what would be the logic of "voting"? i mean would you for example require only people since dan #3 to be able to approve? is it described anywhere how dan allows/restricts ability to manipulate with github or it's TBD?
from runtimes.
Synchronising dans from chain to GitHub teams would also bring an ability to grant other types of GH-permissions (e.g. admin, force-push) to fellows of specific dan.
Isn't as "permissionless" as I proposed, as you would first need to invite someone, but if that is easier, I personally don't care that much. As long as the synchronization would be automatic.
Or only people above some DAN are getting invited as there is more trust in these people.
is it described anywhere how dan allows/restricts ability to manipulate with github or it's TBD?
No, but I also don't see this as that complicated when we have the prerequisites.
from runtimes.
Some things can be done to make this better, for example giving the fellowship itself an identity registrar spot
We are currently working on this.
Not sure why we should maintain multiple lists which will then require someone to update the list in here as well etc.
from runtimes.
@arrudagates do you mean something like 10 approves from people of dan 3 should be equal to 1 approve of dan 6?
from runtimes.
@arrudagates do you mean something like 10 approves from people of dan 3 should be equal to 1 approve of dan 6?
Yes, these ratios would have to be fine tuned, of course, but I think this would be a fair compromise.
Perhaps the ratios could even be dynamic, 75% of total Dan 1 members equals 1 actual approval, then to merge it needs a specific amount of actual approvals
from runtimes.
People can always comment/review any PR without any permissions. It is just that those won't be considered for merge. I have been reviewing and commenting Substrate PR for years, and that's how I participate and rank up myself. I did not need to ask anyone for permissions to start.
Yes, that's fair and it works out for those outside the fellowship, but at the same time those who are already members of the fellowship should have some power regardless of rank (even if weighted lower than upper ranks), otherwise it hurts the fellowship diversity.
from runtimes.
I just want to keep it simple. I hear your request and sounds reasonable but at the same time do you really think it is a must have feature for the initial release? Let's get something out first and then consider improving it. Not the other way around.
from runtimes.
@mordamax what is the status? When do you think we can make use of this bot?
from runtimes.
@mordamax what is the status? When do you think we can make use of this bot?
@Bullrich could you please respond? ☝️
from runtimes.
Hi @bkchr!
Currently review-bot is being built with this in mind. It will have the ability to hot swap the source from where we get the "team" members (currently only GitHub, but it is a replaceable module).
I had been in contact with Harry and also updated @josepot on our needs so we can use CAPI.
Harry built a small function for me where I was able to obtain all the users and metadata from the chain.
We have a blocker which requires a different team to resolve:
While we can obtain the chain data, the data needs to be updated so the fellows add their GitHub handles. I don't know how this is could be done, or who to ask, but until this is resolved, we won't be able to obtain their handles.
Could you please provide me some guidance with this?
from runtimes.
How much time do you think you will need finish 1.0?
Could you please provide me some guidance with this?
Until this issue is done, we will not have "native" support for the github handle. However, we already support additional
data in the identity. We will require to store there ("github", github_handle)
. If the look at the declaration of Data
, we can see that we can at most store up to 32 bytes. github
will fit easily in there as a key
, however the github handle could maybe be longer than 32 bytes. Thus, I would propose that we use BlakeTwo256(GITHUB_HANDLE)
per user there. Then the bot would need to hash the approving accounts as well and check them against this list. Does this sounds reasonable?
from runtimes.
How much time do you think you will need finish 1.0?
Technically speaking I only need to finish 2 more tasks to achieve the same functionality that is being used in the polkadot-sdk
repo.
But, I plan to have all my requirements for version 1 ready by the end of next week (which also includes thorough testing).
Until this issue is done, we will not have "native" support for the github handle.
Thanks for passing me that issue, I'll link it to my tasks.
However, we already support
additional
data in the identity. We will require to store there("github", github_handle)
.
This would work if the users update such additional data.
If the look at the declaration of
Data
, {..} Does this sounds reasonable?
Should be possible, I will need to request the help from the CAPI team, but I don't see any reason for which it wouldn't be achievable.
from runtimes.
This would work if the users update such additional data.
Yes for sure, but this is some I will take care of.
But this is optional? And not really required for the issue here.
from runtimes.
But this is optional? And not really required for the issue here.
Yes, that would only be required to achieve the same functionality that is being used in the polkadot-sdk
repo.
But it is completely optional.
from runtimes.
@Bullrich what is the status?
from runtimes.
Hi @bkchr!
Review-Bot version 1 has been completed yesterday. I'm currently testing complex cases in paritytech-stg
and fixing edge cases.
It currently replicates most of PRCR behaviour (it doesn't assign reviewers yet, that will be in version 1.1.0
).
Once I have this version's bugs fixed, I'll be merging it into polkadot-sdk
.
That's when I'm going to ask @josepot for help with paritytech/review-bot#61
Any updates with the identity field or with paritytech/polkadot-sdk#179?
from runtimes.
We will require to store there
("github", github_handle)
We will start with this, as I said. So, we don't need paritytech/polkadot-sdk#179 for now.
Once I have this version's bugs fixed, I'll be merging it into
polkadot-sdk
.
I'm interested in this repo here and nothing else.
(it doesn't assign reviewers yet, that will be in version
1.1.0
)
We don't need this, we need paritytech/review-bot#61
from runtimes.
@Bullrich it's me again ;) Status?
from runtimes.
Hi @bkchr!
Yesterday @josepot showed us how he is able to fetch the github
handle with his library.
I'm currently close to achieve the same functionality using polkadot-js
(which will be replaced by the other library in the future).
We have been able to extract the github
data in both cases, so we are almost there! I'll comment here once we have the final PR
from runtimes.
@bkchr I have a draft Pull Request that is fetching the fellow's handles here: paritytech/review-bot#79
from runtimes.
I have two more sub tasks:
I will finish those today and have the release ready.
In the meanwhile, do you know what kind of rules will you be requiring?
the config is extremely similar to PRCR, so I can adapt it to review-bot.
from runtimes.
For the pr adding the bot, I propose we let everyone from rank 2 voting. Rank - 1 should be your "voting power". All approvals together should be above 20 voting power. We can then discuss the proper configuration in your pr.
from runtimes.
@Bullrich I see all your tasks are finished? Could you please open a pr to add the bot to the repo?
from runtimes.
@Bullrich I see all your tasks are finished? Could you please open a pr to add the bot to the repo?
@bkchr find the Pull Request here:
I added example cases, but feel free to request the changes that you find suitable.
from runtimes.
Related Issues (20)
- `im-online` session keys migration will not run on Kusama HOT 3
- Ambassador Program Per Referendum 487 HOT 1
- Improve the runtime release process to include debug wasm in artifacts
- Only Release Affected Runtimes
- Remove XCM SafeCallFilter for chains using Weights::v2 (also accounting PoV weight) HOT 1
- Whitelist preimage for Snowbridge initialization HOT 3
- ChaosDAO Community Wishlist HOT 2
- Sending bridged DOT from AH to parachain fails after upgrade HOT 6
- Polkadot block `20,439,207` high mandatory weight HOT 1
- Include Collator Selection Patch to Release Collator DOT/KSM
- Proposing: v1.2.4 Release HOT 10
- Improve backporting releases
- Consider making all polkadot-sdk dependencies workspace dependencies
- Runtime releases management HOT 6
- v1.2.5 Release HOT 12
- Make channel openings to system chains permissionless HOT 1
- frame_support::traits::tokens::fungible user's funds may stuck HOT 7
- Version of `log` is pinned to `0.4.20` HOT 2
- Identify duplicated code and merge them HOT 1
- v1.2.6 Release
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from runtimes.