Skip to content

Add Separate Members Page#64

Merged
mhdawson merged 3 commits intonodejs:masterfrom
pertrai1:members-page
Dec 19, 2018
Merged

Add Separate Members Page#64
mhdawson merged 3 commits intonodejs:masterfrom
pertrai1:members-page

Conversation

@pertrai1
Copy link
Copy Markdown
Contributor

@pertrai1 pertrai1 commented Dec 7, 2018

To keep the README file informative, I believe that the members page
being separate would help to keep a long running list of members from
over-populating the README.

README.md Outdated
The issue will provide schedule logistics as well as
an agenda, links to meeting minutes, and
information about how to join as a participant or a viewer.
<<<<<<< HEAD
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Probably want to clean these up

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good catch @wesleytodd

Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Can you please remove the list from the README? Also add a link to the new file there.

Moreover, can you make the list in alphabetical order while you are at it?

@pertrai1
Copy link
Copy Markdown
Contributor Author

pertrai1 commented Dec 13, 2018

@mcollina @wesleytodd I have fixed README and put the list in alphabetical order.

@pertrai1 pertrai1 force-pushed the members-page branch 3 times, most recently from 11dd4f1 to 12d3e69 Compare December 13, 2018 09:49
@mhdawson
Copy link
Copy Markdown
Member

@pertrai1 any chance you can pull in all of the new members who have open PRs and then we can land this once without having to resolve all of the conflicts?

@pertrai1
Copy link
Copy Markdown
Contributor Author

You better believe it. Get as many PRs in as possible and I will rebase

@pertrai1
Copy link
Copy Markdown
Contributor Author

@mhdawson I have rebased master into this branch with all commits that have been merged. So any PR's still open below this one and above will need to be merged and rebased, or the PR's will have to change with the name being added to MEMBERS.md file.

@pertrai1
Copy link
Copy Markdown
Contributor Author

pertrai1 commented Dec 18, 2018

@mhdawson @mcollina @wesleytodd I have changed the MEMBERS.md to list members using all contributors script to make it look cleaner (background is just from VS code markdown preview). This includes all members up to #86 PR. I get a rate limit of adding members so I need to wait a few hours before I can add the members above this commit.

screen shot 2018-12-18 at 2 12 48 am

@wesleytodd
Copy link
Copy Markdown
Member

This looks great! And the tool for this is also great!

@mhdawson
Copy link
Copy Markdown
Member

Seems like a good idea. How do we handle the case when people move to "Emeritus". I'm not too worried about that now but want to understand how we'd handle that later on?

@wentout
Copy link
Copy Markdown
Contributor

wentout commented Dec 18, 2018

Seems like a good idea. How do we handle the case when people move to "Emeritus". I'm not too worried about that now but want to understand how we'd handle that later on?

I think, as it is rendered HTML, it still can be managed manually. Though, yes, bit harder.

@pertrai1
Copy link
Copy Markdown
Contributor Author

@mhdawson @wentout maybe we don't rely on the script after this first large round of project members is added. All we would do after have copy and paste markdown to add either team member or "Emeritus". Worse case scenario I can think of.

@mhdawson
Copy link
Copy Markdown
Member

@pertrai1 ok sounds reasonable. Once you get it updated let us know so that we can land and close the requests that have been covered.

@pertrai1
Copy link
Copy Markdown
Contributor Author

@mhdawson is there any way I can have access to close PR's as I add a member. I am going over on my limit for the GitHub API having to do this manually. This branch can be merged with all of the people that are listed on current readme as well as some of the PR's that are open. Right now I am getting roughly 5 requests per hour. Let me know what you think.

@ljharb
Copy link
Copy Markdown
Member

ljharb commented Dec 19, 2018

@pertrai1 if you add Closes #123 in your OP, when this PR is merged, it will auto-close them.

@pertrai1
Copy link
Copy Markdown
Contributor Author

@ljharb so I can list a bunch of PR's right after Closes?

@mhdawson
Copy link
Copy Markdown
Member

I'm not quite sure I understand the part about closing the PRs? Does that interact with the tool. What I was thinking is we'd land and update with all of the people who have made requests and then I'd go through and close the PR's one by one to make sure we'd not missed anybody. But I'm also ok using the Closes and then I can double check after the fact with the closed PRs

@ljharb
Copy link
Copy Markdown
Member

ljharb commented Dec 19, 2018

@pertrai1 no, Closes #123. Closes #124. etc.

@pertrai1
Copy link
Copy Markdown
Contributor Author

@mhdawson Right now this has all the current members plus the PR's for 100 and above. Any PR's between this one and #100 have not been added yet. I also changed the readme to mention that someone should open an issue where they want to join. If you prefer to stay with PR's, I will revert that change.

@pertrai1
Copy link
Copy Markdown
Contributor Author

@mhdawson forget where the readme mentions creating issue. The tool works on those who have contributed so PR's are the way to go.

@mhdawson
Copy link
Copy Markdown
Member

Ok, going to land, what is the next step to getting the rest of the PRs between 64 and 100 addressed? Is it as simple as checking out and running to get an update?

@mhdawson
Copy link
Copy Markdown
Member

IT also seems a number above 100 were not added as well. I closed the ones that I think were added.

@wentout
Copy link
Copy Markdown
Contributor

wentout commented Dec 19, 2018

And it also seems like some PRs not counted as contribution.
Cause if we are not merging their PRs, but just closing, github counts as no contribution.

Seems like all who will be added after this, have to update their forks and make additional requests about themselves.

Seems it it necessary to reopen this and others closed after #64

image

all this "Closed" will not count as contribution

I'm telling about this icon

image

Also I think it will be harder now to make some automation, based on github logins. Because we have no simple list, but full HTML, which have to be parsed to extract the logins. So, we can't just copy and paste full list somewhere.

image

all of them are not becoming contributors, cause no merge

@mhdawson, @pertrai1 can I reopen, reslove conflits and merge instead ?

@pertrai1
Copy link
Copy Markdown
Contributor Author

Sounds good to me if it needs to be reopened so we can flush this out

@pertrai1
Copy link
Copy Markdown
Contributor Author

I think once the PR's are merged then generating the new list each time will be much simpler. This first shot was a bit difficult because the PR's were/are not merged yet.

@terary
Copy link
Copy Markdown
Contributor

terary commented Dec 20, 2018

I am one of those non-contributors listed above..
Is there anything I can do to help with this?

@pertrai1
Copy link
Copy Markdown
Contributor Author

@wentout @mhdawson if we can get the conflicts resolved and merge the PR's, then we can generate again and it will correctly add all contributors. Both of you would have to handle the conflicts and merging and I can help out any way possible.

@wentout
Copy link
Copy Markdown
Contributor

wentout commented Dec 20, 2018

@pertrai1 I think it is nice solution. Though still, it requires extra time we have to add new PRs of team membership. If there can be some automation, I'd prefere to have simple list for merging new PRs through which members will receive contribution status, and other beautifull auto-generated list from all-contributors. And still, it looks like too complicated if there will be no automation for this. It requires you to spend your time each member addition. So, it must be automated in another way. Each solution wich spends extra time of one of package-maintenance contributors looks odd for me .

@pertrai1
Copy link
Copy Markdown
Contributor Author

@wentout I completely understand that point of view. Should there still be a separate members file or revert back to using readme?

@wentout
Copy link
Copy Markdown
Contributor

wentout commented Dec 20, 2018

@wentout I completely understand that point of view. Should there still be a separate members file or revert back to using readme?

IDN, seems it is up to community, for sure: @mhdawson, @mcollina, what do you think?
I can try to find solution to automate it via github actions, webhooks or something like this.

@mhdawson
Copy link
Copy Markdown
Member

Ok, I think we should just go back to a simple list at this point. What I'd like to do is to land a single PR which updates the README.md to include all of the members for the existing PRs and then we can take another look at the separate page. @wentout would you be able to put together that PR?

@wentout
Copy link
Copy Markdown
Contributor

wentout commented Dec 20, 2018

@wentout would you be able to put together that PR?

Yes, sure, definetely.

@mhdawson
Copy link
Copy Markdown
Member

@wentout thanks, I'll try to keep an eye out for it so that I can land it soon after you have it ready.

wentout added a commit that referenced this pull request Dec 20, 2018
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.

7 participants