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

Document additional dependencies of importers #468

Merged
merged 6 commits into from
Mar 9, 2022

Conversation

ashmaroli
Copy link
Member

Introduce a rake task to extract list of available importers into a data file that is then consumed during a future jekyll build.
This simplifies maintaining consistency between plugin lib and docs.

Closes #467

Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

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

This is fantastic!! Thanks @ashmaroli! I have a couple quick comments that I'd love your thoughts on to improve this even further.

docs/_layouts/docs.html Show resolved Hide resolved
docs/_data/importer_dependencies.yml Show resolved Hide resolved
- safe_yaml
dotclear:
- active_support
- active_support/core_ext/string/inflections
Copy link
Member

Choose a reason for hiding this comment

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

Should we normalize this to the gem it comes from? I wonder if there's a way to find the file using Ruby's $LOAD_PATH and normalize on the gem name. For example, this and the line above are both from the gem "activesupport", I believe.

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 about it but dropped it for a future pull request..

Copy link
Member Author

Choose a reason for hiding this comment

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

Awesome suggestion regarding the use of $LOAD_PATH.. I did not think of that..

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha. This doesn't quite answer the question of how to install the necessary dependencies for the average end user who isn't that familiar with ruby. You and I know how to install activesupport properly but they may not. I wonder how much this change helps without a more explicit instruction like:

To use this importer, run the following command beforehand:

    gem install activesupport rexml sequel sqlite3

It currently reads to me like a maintainer documentation change rather than an end-user documentation change 🤔

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 see.. I didn't add the gem install so that users do not spend time installing gems they may already have installed..
But yes, your point is valid and makes sense.

@ashmaroli
Copy link
Member Author

Screenshot of current renders

jrnl-deps

pluxml-deps

Rakefile Outdated Show resolved Hide resolved
Rakefile Show resolved Hide resolved
docs/_layouts/docs.html Outdated Show resolved Hide resolved
@ashmaroli
Copy link
Member Author

Latest Render

ghost-deps

@ashmaroli ashmaroli requested a review from parkr March 9, 2022 10:22
@parkr
Copy link
Member

parkr commented Mar 9, 2022

Great work! :shipit:

@parkr
Copy link
Member

parkr commented Mar 9, 2022

@jekyllbot: merge +docs

@jekyllbot jekyllbot merged commit 470db0b into jekyll:master Mar 9, 2022
jekyllbot added a commit that referenced this pull request Mar 9, 2022
@ashmaroli ashmaroli deleted the document-importer-deps branch March 10, 2022 05:16
@jekyll jekyll locked and limited conversation to collaborators Mar 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please add dependencies to Importer docs
3 participants