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

Improve gem documentation #7

Merged
merged 2 commits into from
Feb 27, 2018
Merged

Conversation

kennyadsl
Copy link
Member

@kennyadsl kennyadsl commented Oct 26, 2017

This PR just adds a section in the README that explains how to use new testing helpers for extensions.

It also removes .rb extensions form code documentation requires.

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Thats good by me. I think we improve even more

README.md Outdated
```

`feature_helper` will also require `rails_helper` and `spec_helper`,
which can even be used standalone if needed.
Copy link
Member

Choose a reason for hiding this comment

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

We should also mention that the other helpers are requirable as well. And explain which helper is meant for what tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought this part would explain that:

which can even be used standalone if needed

but I was probably wrong. I'll try to improve it further explaining what each helper is needed for.

@kennyadsl kennyadsl changed the title Update README with testing helper section Improve gem documentation Oct 27, 2017
@kennyadsl
Copy link
Member Author

@tvdeyen done an attempt to improve README following your suggestions. Let me know what do you think!

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Much better. Thanks.

I still think it can be improved so it does not read as if extension developers has to require the rails_helper as well.

README.md Outdated
correctly, setting up Capybara and configuring Rails test application
to precompile assets before the first feature spec.

To run, this helper needs to require a `rails_helper`, also provided by
Copy link
Member

Choose a reason for hiding this comment

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

To run, this helper needs to require a rails_helper, also provided by

This helper requires the rails_helper, also provided by ...

README.md Outdated
like authorization helpers, Solidus core factories, url helpers, and
other helpers to easily work with Solidus Config.

This `rails_helper` in turn requires a basic `spec_helper`, which is
Copy link
Member

Choose a reason for hiding this comment

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

This rails_helper in turn requires a basic spec_helper, which is

This rails_helper in return requires the basic spec_helper, which is

Copy link
Member Author

Choose a reason for hiding this comment

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

I know I can be wrong here but I think in turn here is correct. 😄 🇬🇧

Copy link
Member

Choose a reason for hiding this comment

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

I am also not sure. Leave it as it is then :)

@jhawthorn jhawthorn merged commit f8c5941 into solidusio:master Feb 27, 2018
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.

3 participants