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

Add hooks to doctor #5510

Closed
pathawks opened this issue Oct 23, 2016 · 11 comments
Closed

Add hooks to doctor #5510

pathawks opened this issue Oct 23, 2016 · 11 comments
Assignees
Labels
feature frozen-due-to-age stale Nobody stepped up to work on this issue.

Comments

@pathawks
Copy link
Member

I would love if plugins were able to hook into jekyll doctor. Specifically, I would like for a plugin to be able to add its own method(s) to healthy?.

For a plugin like jekyll-seo-tag it would be nice to have a way of warning users if some recommended attribute is not provided, or if some combination of provided settings results in invalid OGP tags. For jekyll-sitemap or jekyll-feed, it would be nice to alert users if there is already a sitemap or feed present which prevents the plugin from generating one.

This is not something that should break the build, and it's not even really worth raising a warning on each build, but jekyll doctor seems like the right place to put them.

This could even open up a new class of plugins that could scan a site for potential improvements.

/cc @jekyll/ecosystem @jekyll/stability

@envygeeks
Copy link
Contributor

This probably belongs upstream and I also think Jekyll will need a fundamental change for this to happen too. What I mean by belongs upstream is that I think mercenary should handle hooks for commands, we can add our own, that would be neat but hooks are general useful there too so upstream should implement them (if not for us, then as we do for our own commands.)

The fundamental change needs to happen because AFAIW commands are loaded before hooks or plugins (well technically in this case they would be on-in-the same) so we would need to rethink how and what we load and when we load what we load. I could be wrong on this part but every time I think about it and try it, this is the problem.

@pathawks
Copy link
Member Author

The fundamental change needs to happen because AFAIW commands are loaded before hooks or plugins

I think understand what you are saying, but jekyll doctor loads plugins and generates the site. I am just looking for a way for a plugin to provide a method that takes the generated site object, does some stuff with it, maybe logs some messages, and returns a boolean.

@ghost
Copy link

ghost commented Oct 23, 2016

@envygeeks last time i checked, plugins are loaded before mercenary even creates the program

@ghost
Copy link

ghost commented Oct 23, 2016

@fene Bundler plugins are loaded before the command is created, but those from _plugins are loaded by the Jekyll::Site, well after.

@envygeeks it wouldn't be possible to allow plugins from _plugins to act as hooks for a command, since the source folder is not known before the command is constructed, although I don't know if people are too bothered about that.

@ghost
Copy link

ghost commented Oct 23, 2016

@spudowiar ah, that makes sense. thanks for clearing that up

@envygeeks
Copy link
Contributor

it wouldn't be possible

I don't know what this means.

to allow plugins from _plugins to act as hooks for a command, since the source folder is not known before the command is constructed, although I don't know if people are too bothered about that.

I don't know what you mean by "structure" but I do know that it is in fact possible. It's not that hard to decouple and coordinate loading across Jekyll so that commands and their options are loaded, parsing is done and then between parsing and running _plugins are loaded. Our command arguments have _nothing_ to do with a users need to hook into a specific piece of our command, unless they want to hook into lowest level of the command (the arguments) which shouldn't be extensible at all because it'll create confusion.

@ghost
Copy link

ghost commented Oct 25, 2016

@envygeeks You don't know where the site source is before reading command line arguments. Gem based plugins would work fine (using Bundler) as they are loaded before.

@parkr
Copy link
Member

parkr commented Oct 25, 2016

I'd love this. I think we should write the ones we have as hook plugins, too.

I'd love to also look into a Mercenary plugin solution for adding commands and adding other actions to commands in the future, too. I think it's a bit more difficult (due to when/how things are loaded) so I'd rather see a Hook-type solution in the interim. We can deprecate them if we find them to be lacking.

@sgpinkus
Copy link

sgpinkus commented Dec 2, 2016

Sorry to butt in but I'm trying to figure out whether Jekyll supports like pre-build hooks at all (Can't find in doc). Is this discussion "hooks for doctor" or "hooks for Jekyll" in general?

(:+1: for hooks in general if Jekyll don't support already - I want to do JsonSchema validation pre-build..)


Thx for the link @parkr. Sorry about that.

@parkr
Copy link
Member

parkr commented Dec 2, 2016

@sam-at-github You should be able to accomplish that: http://jekyllrb.com/docs/plugins/#hooks

This discussion is just for hooks for the jekyll doctor command.

@jekyllbot
Copy link
Contributor

This issue has been automatically marked as stale because it has not been commented on for at least two months.

The resources of the Jekyll team are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the 3.3-stable or master branch, please reply with all of the information you have about it in order to keep the issue open.

If this is a feature request, please consider building it first as a plugin. Jekyll 3 introduced hooks which provide convenient access points throughout the Jekyll build pipeline whereby most needs can be fulfilled. If this is something that cannot be built as a plugin, then please provide more information about why in order to keep this issue open.

This issue will automatically be closed in two months if no further activity occurs. Thank you for all your contributions.

@jekyllbot jekyllbot added the stale Nobody stepped up to work on this issue. label Jan 23, 2017
@jekyll jekyll locked and limited conversation to collaborators Jul 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature frozen-due-to-age stale Nobody stepped up to work on this issue.
Projects
None yet
Development

No branches or pull requests

5 participants