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

✨ Allow user to specify path to custom README template #68

Merged
merged 13 commits into from
Jun 23, 2019
Merged

✨ Allow user to specify path to custom README template #68

merged 13 commits into from
Jun 23, 2019

Conversation

hgb123
Copy link
Contributor

@hgb123 hgb123 commented Jun 21, 2019

Implementation for #56

But there is a small change in the issue's example, I made it:

npx readme-md-generator -p path/to/my/own/template.md 

instead of

npx readme-md-generator -t path/to/my/own/template.md

Summary of the feature: allow user to specify his own path to the custom template. If the path is not invalid, it will still use the default template.

@codecov
Copy link

codecov bot commented Jun 21, 2019

Codecov Report

Merging #68 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #68   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          21     21           
  Lines         165    175   +10     
  Branches       19     20    +1     
=====================================
+ Hits          165    175   +10
Impacted Files Coverage Δ
src/cli.js 100% <100%> (ø) ⬆️
src/readme.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 688c338...c2dcc5d. Read the comment docs.

@kefranabg
Copy link
Owner

Hi @hgb123,

Thanks for this PR, I'll review it ASAP !

Copy link
Owner

@kefranabg kefranabg left a comment

Choose a reason for hiding this comment

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

Thanks @hgb123 ! Let me know if my thoughts wasn't clear or if you need some help to finish this PR 👍

src/cli.js Outdated Show resolved Hide resolved
src/readme.js Outdated Show resolved Hide resolved
src/utils.js Outdated Show resolved Hide resolved
@hgb123
Copy link
Contributor Author

hgb123 commented Jun 22, 2019

Hi, @kefranabg

I have made some changes with test cases after our discussion above and rebased to resolve conflicts. Please take a look.

Copy link
Owner

@kefranabg kefranabg left a comment

Choose a reason for hiding this comment

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

@hgb123 thanks for your awesome work ! 💪

I just made some changes, I let the error be thrown if template path is not valid, it's better for error tracking. I also added you as a contributor in the README.

Could you check my commit and tell me if you're ok with the changes ?

Copy link
Contributor Author

@hgb123 hgb123 left a comment

Choose a reason for hiding this comment

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

@kefranabg nice refactoring, thank you for working this out with me.

I have leave a comment, please have a look.

src/readme.js Show resolved Hide resolved
@hgb123
Copy link
Contributor Author

hgb123 commented Jun 23, 2019

@kefranabg I have added an example for this in the latest commit. It seems that everything is okay now.

README.md Outdated

```sh
npx readme-md-generator -y
```

Use your custom template (`-p, --path`) (currently we support ejs template only, check our example [here](https://raw.githubusercontent.com/kefranabg/readme-md-generator/master/templates/default.md)):
Copy link
Owner

Choose a reason for hiding this comment

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

Unfortunately, we can't update the documentation for now 😄
I'll update the documentation when I'll publish the new release (4.2.0) on npm.

If we update the doc in this PR this will be displayed for all users but they won't be able to use the -p option as the published version on npm is still the old one

Copy link
Contributor Author

@hgb123 hgb123 Jun 23, 2019

Choose a reason for hiding this comment

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

@yannbertrand Okay. I removed that commit.

Copy link
Owner

@kefranabg kefranabg left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for your work @hgb123 !

@kefranabg kefranabg merged commit e0d66c0 into kefranabg:master Jun 23, 2019
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.

None yet

2 participants