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

Pilot (Student Libraries): Require comments to publish libraries #28870

Merged
merged 2 commits into from Jun 4, 2019

Conversation

jmkulwik
Copy link
Contributor

@jmkulwik jmkulwik commented May 31, 2019

This requires comments on every exported library function.

For reference, this was the original PR that added comments: #28783

requireComments

Spec for reference: https://docs.google.com/document/d/1jMLm_ghCshFen-h9vInAuXwEGXI-HPMjSP8dYeISfM0/edit?pli=1#

This code is largely experimental. If we go forward with Student Libraries, all code here will go through a second CR for production quality review. Review this PR to evaluate the following:

  • Isolation: Will the reviewed code affect parts of the product that are not part of the pilot?
  • Will the code fail in ways that will cause the pilot to be unsuccessful?
  • Is there an obviously easier or better way to implement the piloted feature?

Although the piloting process is still under review, I am following the procedures in this doc: https://docs.google.com/document/d/10QvRAVOTEenFJa1wwrY0Twkd0anTDpkkzNeGnT_HH0o/edit

@joshlory
Copy link
Contributor

joshlory commented Jun 1, 2019

Can you link to the previous PR that added the comment boxes?

@jmkulwik
Copy link
Contributor Author

jmkulwik commented Jun 3, 2019

Can you link to the previous PR that added the comment boxes?

Sure! Added to the description. Also here: #28783

@jmkulwik jmkulwik merged commit 4ba0d0a into staging Jun 4, 2019
jmkulwik added a commit that referenced this pull request Jun 26, 2019
…-comments"

This reverts commit 4ba0d0a, reversing
changes made to ccfaac6.
@jmkulwik jmkulwik deleted the libraries-require-comments branch February 20, 2020 01:34
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.

None yet

2 participants