docs: Add an ADR the format of README files in repos. #363

feanil · 2022-08-01T16:19:56Z

This is an additional ADR for OEP-55 which provides details about the
README specification.

e0d

I've added a couple non-blocking comments.

e0d · 2022-08-01T17:56:10Z

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

+* **Reporting Security Issues** - A link to the e-mail address where security
+  issues can be reported.
+
+Template


Would a link to a template in cookie cutters be better? Worry this will drift compared to where the rubber meets the road.

That's a good call, Once we've hammerd out the contents, I'll update the cookiecutter and link to it from here.

e0d · 2022-08-01T17:57:54Z

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

+  with the component.  It should link to relevant docs as well as places where
+  the users can get help with from other humans.
+
+* **How to Contribute** - A section to indicate how new users can contribute to


Do we need to have a status or similar around whether contributions are desired and/or will be accepted?

I was considering renaming this section to Contributing and then we could let maintainers decide what level of contributions this component is accepting. If we did that, I could imagine us having a status for the level of contribution accepted. Possible values being:

fixes only - no new features are being accepted at this point, but fixes and maintenance updates welcome.

new-features accepted - please discuss your new ideas with the maintainers before you write code to increase the changes that features are accepted.

security only - no changes accepted except for security fixes.

@e0d what do you think, is it worth updating this?

That makes sense to me. Though I think that we should emphasize the following:

contributions should always start with an issue, before code

we prefer maintainers who accept contributions

idegtiarov

One minor nit, the other looks good to me. Thank you.

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

nedbat

It is great that we are codifying this! I've made a few suggestions.

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

nedbat · 2022-08-02T15:33:33Z

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

+* **Purpose** - A paragraph or two summarizing the purpose of this component.
+
+* **Getting Started** - A section to indicate how you can begin development on
+  the new component.


Sometimes Getting Started is about starting to use the component, sometimes it's about starting to develop. You've clearly said "development" here. I wonder if the title of the section should clarify that, or if we might also need how to start using the component?

nedbat · 2022-08-02T15:35:20Z

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

+
+* **People** - A section that tells people how they can find the current
+  maintainers of this component.  Usually a link to the Backstage page for the
+  component and to the catalog-info.yaml file.


Is this section duplicative of Getting Help?

Also, do we need to link to both the Backstage page and the yaml file? Isn't one the published version of the other? I haven't seen what this will look like, does it tell how to actually contact people?

You're right, linking to the component in backstage should be sufficient. I'll update the language.

nedbat · 2022-08-02T15:37:31Z

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

+   The Open edX Code of Conduct
+   ----------------------------
+
+   All community members should familarize themselves with the `Open edX Code of Conduct`_.


"All community members are expected to follow the Open edX Code of Conduct" or something like that.

nedbat · 2022-08-02T15:37:49Z

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

+   ======
+
+   The assigned maintainers for this component and other project details
+   may be found in `Backstage`_ or groked from inspecting catalog-info.yaml.


We aren't going to say "groked", right? :)

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

robrap

Thanks @feanil.

robrap · 2022-08-03T13:45:50Z

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

+Context
+*******
+
+`OEP-55`_ states that one of the jobs of a maintainer is te maintain an


Additional context, is that OEP-19 provided information about READMEs: https://open-edx-proposals.readthedocs.io/en/latest/best-practices/oep-0019-bp-developer-documentation.html#readmes

OEP-19 should get an update, at a minimum to point here for more decisions and details around READMEs. Not sure what else should stay in sync by copying from OEP-19 here, or the other direction.

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

feanil · 2022-08-03T16:46:22Z

Ready for another round of review.

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

robrap

Minor clean-up. Thanks.

robrap · 2022-08-03T20:03:01Z

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

+Status
+******
+
+** Accepted **


Nit: I generally haven't seen formatting for the status.

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

This is an additional ADR for OEP-55 which provides details about the README specification.

Co-authored-by: Igor Degtiarov <degtiarov.igor@gmail.com> Co-authored-by: Ned Batchelder <ned@edx.org> Co-authored-by: Robert Raposa <rraposa@edx.org>

Give maintainers more flexability about the level of contributions they accept and some examples of words they cane use to communicate that to the poor souls who aren't in the know.

robrap · 2022-08-03T21:08:51Z

oeps/processes/oep-0055/decisions/0003-readme-specification.rst

+
+** Accepted **
+
+Context


Oops. @feanil - I didn't mean to approve without #363 (comment) being addressed. It was mistakenly marked as outdated, because a separate change came through on the same line.

Ah, yes, I meant to reply on that one, I just added an update to OEP-55 and OEP-19

Thanks. I think the context paragraph should note the history of addressing READMEs in OEP-19, but I won't belabor the point. I understand it wasn't part of your immediate concern, OEP-55, but it was the initial attempt at codifying the use and purpose of a README.

Now that we have a place to put the specification, update OEPs that reference readmes and link to the specification.

Co-authored-by: Ned Batchelder <ned@edx.org> Co-authored-by: Robert Raposa <rraposa@edx.org>

Co-authored-by: Ned Batchelder <ned@edx.org>

feanil · 2022-08-10T02:28:18Z

FYI, I've incorporated these guidelines into an update of the cookiecutter: openedx/edx-cookiecutters#241 I'll be updating this PR to remove the template readme and point to that one instead.

Link to the cookiecutter version of the template instead of having another copy of the best practices here. This way the cookiecutter copy is treated as the definitive version of the file.

feanil force-pushed the feanil/readme_adr branch from 61e667c to b81bf47 Compare August 1, 2022 16:20

feanil requested a review from e0d August 1, 2022 17:46

e0d approved these changes Aug 1, 2022

View reviewed changes

idegtiarov approved these changes Aug 2, 2022

View reviewed changes

oeps/processes/oep-0055/decisions/0003-readme-specification.rst Outdated Show resolved Hide resolved

nedbat suggested changes Aug 2, 2022

View reviewed changes

robrap reviewed Aug 3, 2022

View reviewed changes

feanil force-pushed the feanil/readme_adr branch from 3baf034 to 96db54a Compare August 3, 2022 16:31

feanil requested review from nedbat and robrap August 3, 2022 16:46

nedbat reviewed Aug 3, 2022

View reviewed changes

oeps/processes/oep-0055/decisions/0003-readme-specification.rst Outdated Show resolved Hide resolved

nedbat reviewed Aug 3, 2022

View reviewed changes

oeps/processes/oep-0055/decisions/0003-readme-specification.rst Outdated Show resolved Hide resolved

nedbat reviewed Aug 3, 2022

View reviewed changes

oeps/processes/oep-0055/decisions/0003-readme-specification.rst Outdated Show resolved Hide resolved

nedbat reviewed Aug 3, 2022

View reviewed changes

oeps/processes/oep-0055/decisions/0003-readme-specification.rst Outdated Show resolved Hide resolved

robrap approved these changes Aug 3, 2022

View reviewed changes

feanil force-pushed the feanil/readme_adr branch from 559b53f to 7174571 Compare August 3, 2022 20:30

Feanil Patel and others added 3 commits August 3, 2022 16:34

docs: Add an ADR the format of README files in repos.

3de5266

This is an additional ADR for OEP-55 which provides details about the README specification.

docs: Apply suggestions from code review

5f482be

Co-authored-by: Igor Degtiarov <degtiarov.igor@gmail.com> Co-authored-by: Ned Batchelder <ned@edx.org> Co-authored-by: Robert Raposa <rraposa@edx.org>

docs: Change the section about contributions.

eab628c

Give maintainers more flexability about the level of contributions they accept and some examples of words they cane use to communicate that to the poor souls who aren't in the know.

feanil force-pushed the feanil/readme_adr branch from 7174571 to eab628c Compare August 3, 2022 20:34

robrap reviewed Aug 3, 2022

View reviewed changes

docs: Update references to READMEs.

268c25f

Now that we have a place to put the specification, update OEPs that reference readmes and link to the specification.

feanil requested review from nedbat and robrap August 3, 2022 21:56

feanil self-assigned this Aug 4, 2022

docs: Apply review suggestions.

d864096

Co-authored-by: Ned Batchelder <ned@edx.org> Co-authored-by: Robert Raposa <rraposa@edx.org>

feanil force-pushed the feanil/readme_adr branch from 8ad52bf to d864096 Compare August 4, 2022 17:42

Feanil Patel and others added 2 commits August 4, 2022 13:44

docs: Also reference OEP-19.

a216559

refactor: Apply suggestions from code review

aaf6cec

Co-authored-by: Ned Batchelder <ned@edx.org>

nedbat approved these changes Aug 5, 2022

View reviewed changes

feanil mentioned this pull request Aug 9, 2022

Land the README ADR for Maintainers openedx/axim-engineering#401

Closed

2 tasks

docs: Replace the README template with a link.

86762b9

Link to the cookiecutter version of the template instead of having another copy of the best practices here. This way the cookiecutter copy is treated as the definitive version of the file.

feanil merged commit 14515df into master Aug 10, 2022

feanil deleted the feanil/readme_adr branch August 10, 2022 19:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: Add an ADR the format of README files in repos. #363

docs: Add an ADR the format of README files in repos. #363

feanil commented Aug 1, 2022

e0d left a comment

e0d Aug 1, 2022

feanil Aug 1, 2022

e0d Aug 1, 2022

feanil Aug 1, 2022

e0d Aug 1, 2022

idegtiarov left a comment

nedbat left a comment

nedbat Aug 2, 2022

nedbat Aug 2, 2022

feanil Aug 3, 2022

nedbat Aug 2, 2022

nedbat Aug 2, 2022

robrap left a comment

robrap Aug 3, 2022

feanil commented Aug 3, 2022

robrap left a comment

robrap Aug 3, 2022

robrap Aug 3, 2022

feanil Aug 3, 2022

robrap Aug 3, 2022

feanil commented Aug 10, 2022


		Accepted

		Context

docs: Add an ADR the format of README files in repos. #363

docs: Add an ADR the format of README files in repos. #363

Conversation

feanil commented Aug 1, 2022

e0d left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

idegtiarov left a comment

Choose a reason for hiding this comment

nedbat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robrap left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feanil commented Aug 3, 2022

robrap left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feanil commented Aug 10, 2022