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

Created base action #1

Merged
merged 6 commits into from
Sep 29, 2023
Merged

Created base action #1

merged 6 commits into from
Sep 29, 2023

Conversation

Bullrich
Copy link
Collaborator

Created the core action to output the fellows GitHub handle.

This action is intended to be used with other actions that needs it (like paritytech/auto-merge-bot in the runtimes repository).

@Bullrich Bullrich self-assigned this Sep 26, 2023
@Bullrich Bullrich requested review from a team and mordamax September 26, 2023 16:43

type FellowData = { address: string; rank: number };

export const fetchAllFellows = async (
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This functionality was copied from review-bot/fellows.ts

README.md Outdated
- `fellows-handles`: A JSON array with all the fellows GitHub handles
- Example: `["user-1","user-2","user-3"]`
- `fellows-map`: A JSON array with objects of type "user": "github-handle", "rank": rank (as a number)
- Example: `[{"user":"user-1",rank: 4}, {"user":"user-2",rank: 2}, {"user":"user-3",rank: 1}]`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's currently mostly about mapping to GH handles, but it could potentially be used in more places and not only using github handles? like mapping their other identity data for sending notifications like email, right?
so im thinking how 'user' would describe that it's a GH username?
may be fellows-identity-map? idk..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will rename this field to be github related so we can do as you suggested in the other comment. Good idea!

logger.debug("Connecting to collective parachain");
// we connect to the collective rpc node
const wsProvider = new WsProvider(
"wss://polkadot-collectives-rpc.polkadot.io",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want later to add more chains besides polkadot/collectives?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say no for now.

If we want, we can create an input type and set it there, but let's not over engineer it right now (as the modification would be simple if we actually need it)

src/index.ts Outdated
return { user: name, rank };
});
setOutput("fellows-map", JSON.stringify(fellowsMap));
setOutput("fellows", fellowsHandles.join(","));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo the action name get-fellows-action sounds to me that it gives just fellow's addresses and ranks maximum.. then when we map it to identity, the identity by itself also consists of many other fields besides Github and if we make it as generic action, it could potentially be re-used somewhere else besides review-bot? but the output assumes the only thing user would need is list of github usernames separated by comma..
i don't know if any other app besides review-bot would benefit from that
or if would benefit - then fellows sounds a bit too broad? or a bit vague in terms of expectations of what kind of data will be there - addresses? or addresses + rank, or full identity map etc

may be it makes sense to provide just one fellows array of objects which complete address,rank,identity data objects? and then consumer converts to any kind of form, whether array of GH handles or anything else

wdyt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that idea, I'll change the current fellows to be fellows-github-handles. Makes more sense

Now the naming makes more sense and it is clearer
@Bullrich Bullrich enabled auto-merge (squash) September 29, 2023 12:20
@Bullrich Bullrich merged commit 581a1e2 into main Sep 29, 2023
4 checks passed
@Bullrich Bullrich deleted the fellow-action branch September 29, 2023 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants