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

Autogenerate README based on description.md #721

Closed
wants to merge 2 commits into from
Closed

Autogenerate README based on description.md #721

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 9, 2017

Why?

According to the docs, README are auto-generated but that is not the case: https://github.com/exercism/ruby#readmes

Manually copy-pasting README from problem-specifications can lead to errors. This creates consistency across all READMEs.

What?

README is generated by combining:

  1. https://github.com/exercism/problem-specifications/blob/master/exercises/accumulate/description.md
  2. https://github.com/exercism/ruby/blob/master/docs/EXERCISE_README_INSERT.md

How Has This Been Tested?

rake test
./bin/generate

Types of changes

  • Add build_readme to generator
  • Add unit tests for generator
  • Run generator across all tests to create uniform READMEs

Checklist:

  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@@ -1,5 +1,3 @@
# Acronym
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have all the titles disappeared?

Copy link
Author

Choose a reason for hiding this comment

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

The titles disappeared because the original problem-specfications/description.md don't have it in them.

Accumulate README from problem-specfications:
https://github.com/exercism/problem-specifications/blob/master/exercises/accumulate/description.md

Ruby's Accumulate README:
https://github.com/exercism/ruby/blob/master/exercises/accumulate/README.md

I was thinking about adding the titles into the description.md of problem-specifications for every problem so that when generator gets run the titles don't disappear.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please have a read through this thread an the associated issues/PRs:
exercism/problem-specifications#767

Copy link
Contributor

Choose a reason for hiding this comment

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

And: https://github.com/Insti/trackler/blob/master/lib/trackler/implementation.rb#L87
For the code that used to (/still does) generate the readmes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Things are a bit more complicated than normal right now as we're trying to make sure that the exercises are compatible with both the existing exercism.io site and the new https://v2.exercism.io/ (nextercism) site that have slightly different requirements.
It is nextercism that requires that the track have local README.md files.

@Insti
Copy link
Contributor

Insti commented Sep 9, 2017

A tool to generate the readmes is good 👍

There was some talk of having configlet generate READMEs, I'm not sure how far that idea progressed. You might like to search the discussions in /meta /configlet /discussions
(there are a lot of repositories!)

@Insti
Copy link
Contributor

Insti commented Sep 9, 2017

I'd like to see better separation of commits here to make this easier to review.
at least:
1 commit for the README generator.
1 commit for the regeneration of all the exercise readmes.
(plus whatever else is relevant, the more atomic commits the better basically.)

@@ -3,7 +3,7 @@
require_relative '../lib/helper'
require 'generator'

paths = Generator::Paths.new(track: EXERCISM_RUBY_ROOT, metadata: METADATA_REPOSITORY_PATH)
paths = Generator::Paths.new(track: EXERCISM_RUBY_ROOT, docs: EXERCISM_RUBY_DOCS, metadata: METADATA_REPOSITORY_PATH)
Copy link
Contributor

Choose a reason for hiding this comment

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

docs: is not an appropriate argument here, the docs path should be derived from the track: EXERCISM_RUBY_ROOT

@@ -1,4 +1,4 @@
# All Your Base
# All_your_base
Copy link
Contributor

Choose a reason for hiding this comment

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

The underscores should not be here.

@@ -0,0 +1,44 @@
# Change
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this showing as completely new?

@ghost
Copy link
Author

ghost commented Sep 9, 2017

Okay, turns out that Configlet actually does the same thing that I was trying to do but it expects the user to run it manually. IE, READMEs are properly generated using:

./configlet generate . --only=sublist

I think a better approach is to add this command to run "confliget generate" command with Travis CI and automatically have READMEs pushed back to repository?

Places to change: https://github.com/exercism/ruby/blob/master/.travis.yml
Upload files after build is successful: https://gist.github.com/willprice/e07efd73fb7f13f917ea

Thoughts?

@Insti
Copy link
Contributor

Insti commented Sep 9, 2017

I think a better approach is to add this command to run "confliget generate" command with Travis CI and automatically have READMEs pushed back to repository?

This is premature I think. How about making the ruby track generate script call out to configlet to regenerate the readme?

@ghost
Copy link
Author

ghost commented Sep 9, 2017

How about making the ruby track generate script call out to configlet to regenerate the readme?

Yeah, that seems more reasonable

@Insti
Copy link
Contributor

Insti commented Sep 9, 2017

If possible do this as 2 pull requests.
1 that just adds the necessary generator changes.
and another that regenerates all the readmes. (You should be able to do this locally without having to have the generator changes merged.)

This will make things easier to review/test/merge.

@Insti
Copy link
Contributor

Insti commented Sep 9, 2017

On a philosophical level, I personally don't think having configlet generate the readmes is the right thing to do, but that's how things seem to be, so we'll go with that for now.

@ghost
Copy link
Author

ghost commented Sep 9, 2017

Calling configlet during generate: #722

@ghost ghost closed this Sep 10, 2017
@ghost ghost deleted the generator_readme branch September 10, 2017 05:16
This pull request was closed.
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.

1 participant