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

Make the meta generator tag optional #481

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

Conversation

melroy89
Copy link

@melroy89 melroy89 commented Jan 22, 2023

Webmasters are able to decide themselves whether they want to include a generate meta HTML tag or not.. seo_tag.skip_generator.

@ashmaroli
Copy link
Member

Prior art: #404

@ashmaroli
Copy link
Member

ashmaroli commented Jan 23, 2023

@Danger89 We, the maintainers, have not reached at a consensus on whether the meta tag ought to be removed completely or rendered conditionally.
Regardless, I'd like to leave one comment as feedback to this pull request:
How exactly are webmasters supposed to opt to render / not render the meta tag? They do not have control over the seotag object from their config file. You have not included any documentation / integration-test to demonstrate usage.

@melroy89
Copy link
Author

melroy89 commented Jan 23, 2023

#404

Is not applying review comments anymore. I applied the patch correctly.

@melroy89
Copy link
Author

melroy89 commented Jan 23, 2023

@Danger89 How exactly are webmasters supposed to opt to render / not render the meta tag? They do not have control over the seotag object from their config file.

Well I think we have control over this. A bit similar to the title (which is also using the seo tag):

https://github.com/jekyll/jekyll-seo-tag/blob/master/docs/advanced-usage.md#disabling-title-output

You have not included any documentation / integration-test to demonstrate usage.

Agreed. I can add that for sure! Hopefully you all find consensus in the meanwhile.

@ashmaroli
Copy link
Member

ashmaroli commented Jan 23, 2023

A bit similar to the title (which is also using the seo tag).

@Danger89 I'm aware of the connection of "title" with {{ seo_tag }}. But have you tested the proposed implementation at your end?
(My previous comment was with respect to then current state of this pull request branch).

@melroy89
Copy link
Author

I will test it soon. And also create the required integration tests as well as user documentation.

@pegvin
Copy link

pegvin commented Mar 23, 2024

I have no clue why this isn't merged yet, it doesn't break anything, and it's a good feature.

@melroy89
Copy link
Author

I have no clue why this isn't merged yet, it doesn't break anything, and it's a good feature that.

Agreed. It's a simple but effective change.

@pegvin
Copy link

pegvin commented Apr 15, 2024

I found a simple fix to this, I just switched to @lumeland, it's far better and faster than jekyll and community support is pretty alive too.

@melroy89
Copy link
Author

melroy89 commented Apr 18, 2024

I found a simple fix to this, I just switched to @lumeland, it's far better and faster than jekyll and community support is pretty alive too.

That is just a weird statement. Just because of this small missing optional meta tag generator? You have still full control over your plugins with Jekyll just like with Hugo. In all cases you can clone the code and made changes yourself, you are in full control how you want to use it or even extend it to your own liking. That being said, I do wish they just merge these PRs, so I don't need to maintain my own fork.

I still need to write an integration test tho..

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

3 participants