Vimux / Binario

Responsive card-based & code-light Hugo theme
https://binario.netlify.app
MIT License
116 stars 53 forks source link

Add tags to entry/meta/tags #62

Closed bugok closed 2 years ago

bugok commented 2 years ago

I would like to enable adding "tags" to Params.Entry.meta list

This is to address issue: https://github.com/Vimux/Binario/issues/61

This is an initial version, and I'd appreciate feedback on how to improve this. I haven't ever done html - so it's very likely that I'm missing something here.

Currently, there are two things which aren't working 100%.

First, I get a test error, but I don't understand why:

0|[noamler@noamler-mbp]:Binario (tags_meta)$ npm test

> binario@1.0.0 test
> npm run lint

> binario@1.0.0 lint
> npm run lint:css && npm run lint:js && npm run lint:editorconfig

> binario@1.0.0 lint:css
> stylelint static/css/*.css assets/css/*.css

> binario@1.0.0 lint:js
> eslint static/js/*.js

> binario@1.0.0 lint:editorconfig
> editorconfig-checker

layouts/partials/entry/meta/tags.html:
    No final newline expected

1 errors found

I don't see that there's a newline at the end of the tags.html file - so I'm not sure why this is happening.

Second, the Tags section in the summary isn't starting in a new line (see screenshots). I'm not sure how to fix that. Screen Shot 2022-03-06 at 16 36 52 Screen Shot 2022-03-06 at 16 36 57

I tested this using a blog of mine - pointing the themes/binario link to be on my local repo instead of the git submodule. I'd be happy to test otherwise.

bugok commented 2 years ago

I see that build is failing.

Seems like :

bugok commented 2 years ago

(fixed my editor to not include the terminating '\n')

Vimux commented 2 years ago

Sorry, I don't have time right now. I'll try to take a look this weekend, but I can't guarantee that either. Thank you for your patience.

bugok commented 2 years ago

i18n-warnings was renamed to printI18nWarnings here: https://github.com/gohugoio/hugo/commit/837fdfdf45014e3d5ef3b00b01548b68a4489c5f

bugok commented 2 years ago

Tests are failing because of a version issue in the CI, from what I can tell. https://github.com/Vimux/Binario/pull/63 should fix it.

bugok commented 2 years ago

I messed up my local branch. Hence the pushes. Will follow up with the suggested changes.

bugok commented 2 years ago

Here's a screenshot with local testing of my personal blog. In the screenshot you can see how entries look:

Params.Entry.meta is configured with ["date", "categories", "tags"] Screen Shot 2022-03-15 at 20 14 24

bugok commented 2 years ago

This is unrelated to this specific issue, but more looking for general guidance: when I get a review, is it better to send another commit with only the required changes, or is it better to amend the ongoing commit like I've been doing?

At my workplace, I'd usually send different commits, but I'm not sure how this works in the real world.

Thanks!

Vimux commented 2 years ago

One more little thing. Could you please remove or rephrase the commit description? Not a big deal, just my perfectionism.


Regarding your question, I'm not sure if you're asking in general or specifically about my repositories.

My colleagues and I leave all the commits mostly. Public GitHub is a different place. I'm not a good role model because open-source is one of my hobbies, not a job. In the world of unicorns and ponies, I (as a maintainer) would prefer clear commit history with good titles and descriptions without forcing, which is difficult to achieve in the real world. In addition, sometimes it is not necessary but takes time. If unsure, see the README and Contributing files, plus the last 3-5 successful PRs with multiple rounds of reviews as a reference.

I accept both options in public from contributors. All in all, this is a philosophical question.

Vimux commented 2 years ago

LGTM, thank you.

P.S. I hope my nitpicking wasn't too annoying for you.

bugok commented 2 years ago

All good. I don't mind at all. This is your repo and your rules. I didn't find your comments too harsh.