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

Turn anatole theme into a hugo module #291

Merged
merged 2 commits into from
Jan 14, 2022
Merged

Conversation

deining
Copy link
Contributor

@deining deining commented Jan 7, 2022

This PR turns the theme into a hugo module.
With gitand go installed, making use of the anatole theme is now as easy as:

hugo new site my_site
cd mysite
echo 'theme = "github.com/lxndrblz/anatole"' >> config.toml
hugo server

Copying/cloning the theme in the themes folder is still possible of course.

If desired, I can extend the Quickstart section in README.md, explaining how to make use of the theme as module.

@netlify
Copy link

netlify bot commented Jan 7, 2022

✔️ Deploy Preview for anatole-demo ready!

🔨 Explore the source changes: c93fa8b

🔍 Inspect the deploy log: https://app.netlify.com/sites/anatole-demo/deploys/61df3e3997409f0008bf4275

😎 Browse the preview: https://deploy-preview-291--anatole-demo.netlify.app

@lxndrblz
Copy link
Owner

lxndrblz commented Jan 7, 2022

@deining Thanks for the PR.

Would you mind adding a brief explanation to the QuickStart section of the Readme?

@deining deining force-pushed the module branch 4 times, most recently from a7165ba to 5fce61d Compare January 8, 2022 20:07
@deining
Copy link
Contributor Author

deining commented Jan 8, 2022

Would you mind adding a brief explanation to the QuickStart section of the Readme?

I just added a commit that adds documentation on how to use the theme as module.
Please let me know if you like it. I proposed using modules as recommended approach, but I'm certainly biased here.

Copy link
Owner

@lxndrblz lxndrblz left a comment

Choose a reason for hiding this comment

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

@deining Thanks so much for your PR. Everything works as expected and it is good to have an alternative approach in addition to the current way of adding the theme.

Personally, I am a bit torn, whether we should add the new way really as Route 1 in the readme, as most users might not have go installed but simply rely on the executable for creating and building the site. Therefore, I would probably flip route 1 and 2 as this still seems to be the prevalent approach in the community. What do you think?

@deining
Copy link
Contributor Author

deining commented Jan 12, 2022

@deining Thanks so much for your PR. Everything works as expected and it is good to have an alternative approach in addition to the current way of adding the theme.

Glad to hear that.

Personally, I am a bit torn, whether we should add the new way really as Route 1 in the readme, as most users might not have go installed but simply rely on the executable for creating and building the site. Therefore, I would probably flip route 1 and 2 as this still seems to be the prevalent approach in the community. What do you think?

I intentionally put the hugo approach in first place. Having worked with modules for a while now, I think they are superior to git submodules. Admittedly, the difference is not that big, handling themes the traditional way is certainly a reasonable approach. You may read this blog post, I fully agree with what is stated there. Also, I see that hugo maintainers clearly go for the module approach (see Ananke theme, DocuApi theme, ...). Please also note that official hugo docs dropped section on Installing and using themes in favour of a section on Hugo modules. And yes, your are certainly right, route 2 is currently the prevalent approach in the community. So it's up to you if you want to go mainstream or be innovative 😉 .

@lxndrblz, you are the maintainer of this theme, it's completely your decision. If you decide to flip route 1 and route 2, we will do that. Let me know what you flip route 1 and route 2 and if so, if you want me to do it or do it by yourself. In any case, in go.mod, we should set minimum go version to 1.12 (it's 1.17 right now).

Thanks for reviewing and accepting my PR (with whatever route order, it's up to you).

@deining deining requested a review from lxndrblz January 12, 2022 13:29
@lxndrblz
Copy link
Owner

@deining Thanks for the links. After reading through and reviewing them, I agree that the go based approach should be the preferred way forward.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@deining
Copy link
Contributor Author

deining commented Jan 12, 2022

@deining Thanks for the links. After reading through and reviewing them, I agree that the go based approach should be the preferred way forward.

Great!

I set the minimum version in both go.mod files to 1.12. Any further issues that should be adressed? From my point of view, my PR is ready for merging now.

Copy link
Owner

@lxndrblz lxndrblz left a comment

Choose a reason for hiding this comment

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

@deining LGTM. Thanks for the PR!

@lxndrblz lxndrblz merged commit fcbc65c into lxndrblz:master Jan 14, 2022
@deining deining deleted the module branch January 14, 2022 16:21
@lxndrblz lxndrblz added the enhancement New feature or request label Mar 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants