Add basic readme generator #580

theacodes · 2016-10-13T22:32:20Z

(Only used for storage, but once in I'll start adding other products)

theacodes · 2016-10-13T22:32:26Z

jerjou · 2016-10-13T22:34:40Z

We don't want this to be part of dpebot's functionality? Seems a bit meta to put into the repo itself
rst > md?

theacodes · 2016-10-13T22:37:47Z

We don't want this to be part of dpebot's functionality? Seems a bit meta to put into the repo itself

Nah, at least not for now. The templates and generator are pretty python-specific right now. DPEbot could handle running nox -s readmegen on a schedule and sending PRs as needed.

rst > md?

Many Python projects use rst and github supports both (yay). I personally prefer rst.

jerjou

This seems fine, technically (though you might want to add tests, if we're going to be using this extensively).

I'm concerned that this is overengineering the problem, though. eg what are the pros and cons of this approach, vs having a template directory that you can copy and modify when you create a new sample?

My take:

Pro: common templates for auth, setup - easier management for commonly-repeated instructions
Con: Another management system to learn and maintain

It sort of feels like this is a tool to make github READMEs into a CMS...

jerjou · 2016-10-14T00:11:51Z

nox.py

@@ -320,3 +320,12 @@ def mark_if_necessary(requirement):
        for requirement in sorted(requirements, key=lambda s: s.lower()):
            if not requirement.startswith('#'):
                f.write(requirement)
+
+
+def session_readmegen(session):


...read-memegen 😏

jerjou · 2016-10-14T00:12:46Z

nox.py

+def session_readmegen(session):
+    session.install('-r', 'testing/requirements-dev.txt')
+
+    in_files = list(list_files('.', 'README.rst.in'))


Why not a .yaml extension?

.in indicates that the file is used to generate the file of the same name before it, so README.rst.in -> README.rst, whereas README.yaml.in -> README.yaml.

Oh - I meant README.rst.yaml to make clear that it's a yaml file (and for syntax highlighting, etc). The fact that it's an input to the resulting rst would be implied.

I think .in is clearer that it's used exclusively for generating the README.rst file, but I'm happy to be disagreed with. @waprin, thoughts?

jerjou · 2016-10-14T00:13:20Z

scripts/readme-gen/readme_gen.py

+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+


A module-level docstring would be nice

Added a simple one for now.

jerjou · 2016-10-14T00:13:25Z

scripts/readme-gen/readme_gen.py

@@ -0,0 +1,63 @@
+#!/usr/bin/env python
+
+# Copyright (C) 2013 Google Inc.


jerjou · 2016-10-14T00:20:51Z

scripts/readme-gen/readme_gen.py

+    loader=jinja2.FileSystemLoader(
+        os.path.abspath(os.path.join(os.path.dirname(__file__), 'templates'))))
+
+README_TMPL = jinja_env.get_template('README.tmpl.rst')


Suggestion: we could adopt the rails convention of ordering the file extensions by processing order? ie README.rst.jinja2, where it'd get processed by jinja2 first, then as an rst

jerjou · 2016-10-14T00:25:53Z

scripts/readme-gen/templates/README.tmpl.rst

@@ -0,0 +1,60 @@
+.. This file is automatically generated. Do not edit this file directly.


Possibly add a jinja2 comment that's like:

{# The following line is a lie. BUT! Once jinja2 is done with it, it will become truth! #}

Done (verbatim)

waprin · 2016-10-14T00:47:27Z

scripts/travis.sh

-     nox --stop-on-first-error -s lint travis;
+    source ${TRAVIS_BUILD_DIR}/testing/test-env.sh;
+    export GOOGLE_APPLICATION_CREDENTIALS=${TRAVIS_BUILD_DIR}/testing/service-account.json
+    nox --stop-on-first-error -s lint travis;


I am not a purist if you want to sneak this in there, but it does look like this should be a separate PR? Or is this somehow related and I'm missing something?

Yeah, I need to rebase. Just consider this part of the diffbase.

waprin · 2016-10-14T00:48:41Z

Overall very nice idea, writing the README is a pain and good to standardize on our texts etc. Might be helpful to add a quick usage doc on using the README generator?

Change-Id: I53b753fb7e422161b443cd6d96df9501a7b37ab4

theacodes · 2016-10-14T17:06:55Z

I'm concerned that this is overengineering the problem, though. eg what are the pros and cons of this approach, vs having a template directory that you can copy and modify when you create a new sample?

The pros of this approach is knowing that everything is always consistent. The cons of course are that it is a bit over-engineered and might be hard to maintain down the road, and it might be difficult to accommodate special situations (like speech's portaudio dependency). If we just have a template directory we will inevitably forget to update READMEs after their initial creation.

I could also go the other direction, like we did with auto-docs-links. Instead of this generating READMEs, it could just fill in sections of READMEs automatically:

README.rst

Blah Blah Blah

<!-- include: auth --><!-- end -->
<!-- include: install_deps --><!-- end -->

Then when you run the generator/filler:

Blah Blah Blah

<!-- include: auth -->
Blah Blah
<!-- end -->

<!-- include: install_deps -->
Blah Blah
<!-- end -->

Might be helpful to add a quick usage doc on using the README generator?

Let me get it working across several more samples and then I'll add a README.rst to the README generator. ;)

pfritzsche · 2016-10-14T17:11:25Z

theacodes · 2016-10-14T17:12:49Z

@pfritzsche should readme-gen generate its own readme and read memegen at once?

Change-Id: I67bd8a38f39feb78c669c4e5b25e0293094c10be

pfritzsche · 2016-10-14T17:15:24Z

scripts/readme-gen/templates/auth.tmpl.rst

++++++++++++++
+
+Authentication is typically done through `Application Default Credentials`_,
+this means you do not have to change the code to authenticate as long as


Sentence fragment. Consider:

Authentication is typically done through Application Default Credentials, which...

instead of "this"

jerjou

*shrug
Have at it!

jerjou · 2016-10-14T17:10:52Z

nox.py

+def session_readmegen(session):
+    session.install('-r', 'testing/requirements-dev.txt')
+
+    in_files = list(list_files('.', 'README.rst.in'))


Oh - I meant README.rst.yaml to make clear that it's a yaml file (and for syntax highlighting, etc). The fact that it's an input to the resulting rst would be implied.

Change-Id: Ie33893bd2a1f36a34ab7dbb14c01425c9d0eebb5

theacodes · 2016-10-14T17:26:35Z

@jerjou thanks! I'd like to see how @waprin thinks about it first.

theacodes · 2016-10-18T20:41:42Z

@waprin ping?

waprin · 2016-10-18T20:49:39Z

scripts/readme-gen/templates/auth.tmpl.rst

+
+    .. code-block:: bash
+
+        gcloud beta auth application-default login


Think the command is out of beta

waprin · 2016-10-18T20:52:14Z

storage/cloud-client/README.rst.in

+samples:
+- name: Snippets
+  file: snippets.py
+  show_help: true


maybe show_help defaults to true?

waprin · 2016-10-18T20:55:27Z

LGTM

theacodes assigned jerjou, waprin and ryanmats Oct 13, 2016

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 13, 2016

jerjou reviewed Oct 14, 2016

View reviewed changes

waprin reviewed Oct 14, 2016

View reviewed changes

Add basic readme generator

d2d586f

Change-Id: I53b753fb7e422161b443cd6d96df9501a7b37ab4

Address review comments

49b0318

Change-Id: I67bd8a38f39feb78c669c4e5b25e0293094c10be

theacodes force-pushed the readme-script branch from b81b3ae to 49b0318 Compare October 14, 2016 17:14

pfritzsche reviewed Oct 14, 2016

View reviewed changes

jerjou approved these changes Oct 14, 2016

View reviewed changes

Fix sentence fragment

fc57873

Change-Id: Ie33893bd2a1f36a34ab7dbb14c01425c9d0eebb5

waprin reviewed Oct 18, 2016

View reviewed changes

theacodes merged commit 530ab8a into master Oct 20, 2016

theacodes deleted the readme-script branch October 20, 2016 17:01

		@@ -0,0 +1,63 @@
		#!/usr/bin/env python

		# Copyright (C) 2013 Google Inc.

		@@ -0,0 +1,60 @@
		.. This file is automatically generated. Do not edit this file directly.


		.. code-block:: bash

		gcloud beta auth application-default login

Add basic readme generator #580

Add basic readme generator #580

Uh oh!

Conversation

theacodes commented Oct 13, 2016

Uh oh!

theacodes commented Oct 13, 2016

Uh oh!

jerjou commented Oct 13, 2016

Uh oh!

theacodes commented Oct 13, 2016

Uh oh!

jerjou left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

waprin commented Oct 14, 2016

Uh oh!

theacodes commented Oct 14, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pfritzsche commented Oct 14, 2016

Uh oh!

theacodes commented Oct 14, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerjou left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theacodes commented Oct 14, 2016

Uh oh!

theacodes commented Oct 18, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

waprin commented Oct 18, 2016

Uh oh!

Uh oh!

theacodes commented Oct 14, 2016 •

edited

Loading