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

Store list of collaborators and TSC in a JSON file #23623

Closed
trivikr opened this issue Oct 12, 2018 · 16 comments
Closed

Store list of collaborators and TSC in a JSON file #23623

trivikr opened this issue Oct 12, 2018 · 16 comments

Comments

@trivikr
Copy link
Member

trivikr commented Oct 12, 2018

Is your feature request related to a problem? Please describe.
As mentioned by @joyeecheung during CollabSummit, node-core-utils reads list of collaborators and TSC members from README.md

Describe the solution you'd like
It would be helpful if we have the list stored in JSON, and is read from all places where it's required.

@joyeecheung
Copy link
Member

This can also help other projects to reuse node-core-utils if they just have a similar list of collaborators

Refs: nodejs/llnode#242

@ChALkeR
Copy link
Member

ChALkeR commented Oct 12, 2018

Is it supposed to be auto-generated from README.md, or is it supposed to be maintained manually?
I would prefer the former.

@joyeecheung
Copy link
Member

@ChALkeR I guess it wouldn't be too hard to auto-generate this considering we have the parser already in https://github.com/nodejs/node-core-utils/blob/master/lib/collaborators.js and the JSON.stringify basically just looks like https://github.com/nodejs/node-core-utils/blob/master/test/fixtures/collaborators.json

@joyeecheung
Copy link
Member

joyeecheung commented Oct 12, 2018

The problem is...who runs the generator? Should we make it part of the onboarding process...?

@joyeecheung
Copy link
Member

joyeecheung commented Oct 12, 2018

Or, should we generate things the other way around? (JSON -> markdown)? At least I personally find it easier to fill a JSON object than to fill a markdown row with a bunch of markers, and put it into the right place alphabetically..

@refack
Copy link
Contributor

refack commented Oct 12, 2018

I'd say store the json in an HTML comment inside README.md then at onboarding, update the object, then run the generator, then commit.

@richardlau
Copy link
Member

Is there a reason ncu can't be pointed at the relevant GitHub teams and retrieve via the API?

@refack
Copy link
Contributor

refack commented Oct 13, 2018

Is there a reason ncu can't be pointed at the relevant GitHub teams and retrieve via the API?

I can think of two minor reasons.

  1. Teams visibility is limited.
  2. It's an API call.

@sagitsofan
Copy link
Contributor

sagitsofan commented Oct 13, 2018

The problem is...who runs the generator? Should we make it part of the onboarding process...?

  1. Does this JSON file needs to be part of the node-core-utils project or it can be under the node project?

  2. Why not running this collaborators generator whenever the api document generator run?

@joyeecheung
Copy link
Member

Is there a reason ncu can't be pointed at the relevant GitHub teams and retrieve via the API?

It's not necessarily true that the name and the email from people's GitHub profile match the name and the email that they want to use in this project. Also there's no pronouns in GitHub profiles.

@joyeecheung
Copy link
Member

Does this JSON file needs to be part of the node-core-utils project or it can be under the node project?

I'd say it should be in the project otherwise you'll need to submit PRs to ncu to add yourself to the list of collaborators, which kind of misses the point of onboarding - landing a PR in core.

@richardlau
Copy link
Member

Is there a reason ncu can't be pointed at the relevant GitHub teams and retrieve via the API?

It's not necessarily true that the name and the email from people's GitHub profile match the name and the email that they want to use in this project. Also there's no pronouns in GitHub profiles.

.mailmap? And why does ncu need to know pronouns?

@joyeecheung
Copy link
Member

joyeecheung commented Oct 13, 2018

@richardlau

.mailmap?

That probably works as well if we regenerate the current .mailmap to include people's github handles (not everyone makes their email available in their GitHub profile so you can't get emails from handles, it could be some no-reply email or null, and there is no guarantee that you can get handles from emails either)

And why does ncu need to know pronouns?

Was talking about generating markdown from JSON (or another file like .mailmap) and make that file the source of truth.

@Trott
Copy link
Member

Trott commented Nov 16, 2018

Is there a clear first step to take here?

@joyeecheung
Copy link
Member

joyeecheung commented Nov 19, 2018

@Trott I personally think it would be more efficient if we just try to solve #24117 - and the "list of collaborators" would just be a subset of the data requested there, if, say, we add the concept of committers to the metadata, so there will be

  • Owners: team(s) that "owns" certain files/folders and are thus likely reviewers
  • Committers: team(s) who have write access to these files - for this repo it's "collaborators or else" but we can reuse a format for other repos
  • Observers: team(s) who need to be notified when these files are changed in a patch

@joyeecheung
Copy link
Member

Merged into #24117

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

No branches or pull requests

7 participants