Skip to content

Markdown docs from source files #9

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

petrbela
Copy link

Still WIP, opening the PR for feedback on design changes I made for the sake of clarity/simplification.

Also would like to add tests before merging (currently have them almost working locally).

@matteodepalo
Copy link

Wow this is amazing, thank you!
Code looks good, however I feel like the rspec_api_blueprint.rb file is getting a bit too big now, with long, non encapsulated methods. It started out as a simple script but I think it's time to refactor it. A possible approach would be to create a Parser and Writer class that would get instantiated by the runner after(:each). This would make testing and maintaining it easier. Thoughts?

@kpgdev
Copy link

kpgdev commented Jan 27, 2014

This is great! Good work... Okay so to get things working I had to:

  1. add 'require 'rspec/rails'' at the top of rspec_api_blueprint.rb
  2. set 'response = @response' & 'request = @request' in config.after (not sure why these aren't being set).

going to work some more on it tomorrow. do you want to commit?

@petrbela
Copy link
Author

@matteodepalo That makes sense, I'll refactor it.

@kpgdev

  1. Worked for me in a Rails app without explicitly requiring it but it shouldn't do any harm.
  2. What env are you using? I had to add last_request/last_response for the same reason, not sure what other variables rspec exposes.

@kpgdev
Copy link

kpgdev commented Jan 28, 2014

i am using ENV='test'

a couple of things to note…

  1. it's not picking up all models. Errno::ENOENT: No such file or directory - app/models/checkout.rb…. as an example - i am using spree and its not finding all the models with the gems. the config only specifies one path. can you get this from rails? just wondering if there's an internal rails config/ internal array that specifies all the model paths?

  2. the markdown doesn’t seem to be aggregating into one file although they are still being generated even though the tests are failing (which is great).

let me know if there’s anything you would like me to test.

@petrbela
Copy link
Author

  1. You can pass a Proc in the Configuration to determine the model name, however checkout should be guessed automatically if you name the example group Group Checkout. Don't know if rails provides any helper but I think it just determines the string using the Inflector.

  2. It currently doesn't concat the generated files into one. I have a separate ruby script to do that but might consider including it as an option (or a default) in the plugin itself.

@matteodepalo
Copy link

For 2) I'd like to see it as an option. Keeping separate files helps readability and caching.

in_comment = false
comment_lines = []

File.open(File.join(folder, file_name.pluralize.underscore + '_controller.rb'), 'r').each do |line|

Choose a reason for hiding this comment

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

There is an issue with this because if you put your controllers into a separate folder, for example app/controllers/api it blows up.

Copy link
Author

Choose a reason for hiding this comment

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

That's what the Configuration is for.

However, I'll be happy with a better way to determine the file location. One option is to take it from the described_class if the spec is written as

describe Api::ArenasController do
  ...
end

which would work for the controller (action) docs. Not sure how to leverage this for models although they are usually all in one folder even if you have multiple api versions.

Choose a reason for hiding this comment

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

Good point, didn't notice that config option.
I don't think there is a need to be smarter about determining the location since in request specs you usually write describe 'Arenas Requests' instead of specifying the controller class.

Copy link
Author

Choose a reason for hiding this comment

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

Well, about Arenas Requests... I changed it to read as Group Arena since that's what is then generated into the blueprint and I'm not sure ... Requests is a rspec (community) standard.

Choose a reason for hiding this comment

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

Yeah in any case it's usually strings.

@kurko
Copy link

kurko commented Jan 11, 2015

Anyone still interested in this? I'd love to see this become reality.

@petrbela
Copy link
Author

We're actively using it internally (latest changes at https://github.com/chute/rspec_api_blueprint/tree/feat/markdown-docs).

Will try to find some time probably next weekend to refactor the code a finish the PR.

@kurko
Copy link

kurko commented Jan 17, 2015

I was going to pull and rebase this, but it's going to be tough because the PR has no tests :(

@petrbela
Copy link
Author

I started writing some tests on my branch, let me review and push them.

@ChrisCPO
Copy link

I just started using this gem, I see the dates of this comment is this gem still being maintained?

@petrbela
Copy link
Author

@ChrisCPO Not very much maintained although it's still being used. You can pull the latest from https://github.com/chute/rspec_api_blueprint/tree/feat/markdown-docs but it comes with no warranty ;)

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.

5 participants