Skip to content
This repository has been archived by the owner on Nov 16, 2022. It is now read-only.

Revise review howto in light of ToS changes #861

Merged
merged 3 commits into from
Dec 12, 2016
Merged

Revise review howto in light of ToS changes #861

merged 3 commits into from
Dec 12, 2016

Conversation

chadwhitacre
Copy link
Contributor

@chadwhitacre chadwhitacre commented Oct 22, 2016

Part of #432.

Closes #803.

@chadwhitacre chadwhitacre added this to the Decouple milestone Oct 22, 2016
@chadwhitacre
Copy link
Contributor Author

Ready for review, @mattbk!

@chadwhitacre
Copy link
Contributor Author

screen shot 2016-10-22 at 4 12 05 pm

@mattbk
Copy link
Contributor

mattbk commented Oct 22, 2016

Should we have some sort of note in there about how we used to require open work, since it's discussed in the project-review repo?

Also, this has to wait for gratipay/gratipay.com#4117 to merge, correct?

@chadwhitacre
Copy link
Contributor Author

since it's discussed in the project-review repo?

You mean in the project-review readme, or in tickets? If the former, let's take it out; if the latter, do you really think we need to worry about it? Is the concern that someone might read an old ticket and wonder what we're talking about?

@chadwhitacre
Copy link
Contributor Author

Also, this has to wait for gratipay/gratipay.com#4117 to merge, correct?

Yeah, I think we should queue up all of these PRs, and then merge and deploy them as close together as we can.

@chadwhitacre
Copy link
Contributor Author

these PRs

Namely, the ones under todo on #432.

@mattbk
Copy link
Contributor

mattbk commented Oct 23, 2016

In tickets. The concern is that someone might read old tickets in order to understand whether their project fits in...well, never mind. We have new TOS that covers everything. If people are confused they can ask, or look at an old version of this page.

I approve these changes.

@chadwhitacre
Copy link
Contributor Author

On the other hand, new folks who read the docs and see a note about the old way will also be confused. In general I think it's better to let the past be in the past.

frozen-image-frozen-36220509-500-209

@mattbk
Copy link
Contributor

mattbk commented Oct 23, 2016

No worries. At least I know about it, so I can effectively support. ;)

@chadwhitacre
Copy link
Contributor Author

Well, but this document is aimed at internal staff such as yourself. It's fair to add a line giving context for old review tickets.

@chadwhitacre
Copy link
Contributor Author

Do we need to think about project review for npm packages (gratipay/gratipay.com#4148) now? Or do we punt on that?

@chadwhitacre
Copy link
Contributor Author

Or do we punt on that?

Punt, I think.

@chadwhitacre
Copy link
Contributor Author

In which case I am good to go here. How's it look to you, @mattbk et al.?

@mattbk
Copy link
Contributor

mattbk commented Dec 9, 2016

Most recent changes LGTM.

@chadwhitacre chadwhitacre merged commit a4c4cde into master Dec 12, 2016
@chadwhitacre chadwhitacre deleted the projects branch December 12, 2016 23:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants