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

kbn-handlebars dependency update #613

Merged

Conversation

toasted-nutbread
Copy link

In lieu of elastic/kibana creating a npm package, how about we do something like this for this dependency? At the very least, this takes its code out of the repo and offloads any maintenance into a separate repo. Right now, it's referencing a repo that I set up, but it should realistically be something that is in themoeway's organization. If this works out, let me know if I can help with that.

Something else I've thought of a few times is that if it's possible to make handlebars support async functions, a lot of the Anki note generation process could be able to be simplified. The fact that it's limited right now to being synchronous causes a lot of problems that make it difficult to add and maintain certain functionality. So that may lend to some justification of just hosting a fork of kbn-handlebars and potentially investigating to see if changes could be made to it that would help specifically this project.

This presumably resolves #525.
Should allow #529 to work.

@toasted-nutbread toasted-nutbread requested a review from a team as a code owner February 2, 2024 22:45
Copy link

github-actions bot commented Feb 2, 2024

✔️ No visual differences introduced by this PR.

View Playwright Report (note: open the "playwright-report" artifact)

@djahandarie
Copy link
Collaborator

Agreed that this is cleaner. I invited you as a member to themoeway github org, so once you accept that, you should be able to transfer the repo. I will also make sure the branch protections on it are set up properly, as that is basically our main security control for everything we do, and I'd like to make sure it's set up for all yomitan-related projects.

@toasted-nutbread
Copy link
Author

Okay I've transferred the repo and updated the refs. I also updated the readme a little, but it's a little bit hard to attribute the same way as the others since it's a custom fork of a sub-folder of a mono-repo.

@djahandarie
Copy link
Collaborator

Thanks! Is that the output of npm run license-report-markdown or did you do it by hand? (I have no idea, as I've never seen its output for things pinned by hash)

@toasted-nutbread
Copy link
Author

It's not, I didn't realize that's how that was generated. This is the output:

Name Installed version License type Link
@zip.js/zip.js 2.7.32 BSD-3-Clause git+https://github.com/gildas-lormeau/zip.js.git
dexie 3.2.4 Apache-2.0 git+https://github.com/dfahlander/Dexie.js.git
dexie-export-import 4.0.7 Apache-2.0 git+https://github.com/dexie/Dexie.js.git
yomitan-handlebars 1.0.0 MIT n/a
parse5 7.1.2 MIT git://github.com/inikulin/parse5.git
wanakana 5.3.1 MIT git+ssh://git@github.com/WaniKani/WanaKana.git

Which is not super meaningful for it, as I think it's always been version 1.0.0, and the n/a comes from me just directly cloning the folder contents without any metadata setup in package.json

@djahandarie
Copy link
Collaborator

@toasted-nutbread I think I'd prefer just sticking to the output of that command even if it's kind of useless. I think we should optimize for maintainability than having perfect legal references.

That said feel free to do whatever you want, I'll merge. (Just needs a merge conflict fix at least)

Copy link

codspeed-hq bot commented Feb 4, 2024

CodSpeed Performance Report

Merging #613 will not alter performance

Comparing toasted-nutbread:kbn-handlebars-dependency-update (111592c) with master (6b327e0)

Summary

✅ 1 untouched benchmarks

@djahandarie djahandarie added this pull request to the merge queue Feb 5, 2024
Merged via the queue into yomidevs:master with commit 71c3aff Feb 5, 2024
8 checks passed
@djahandarie djahandarie added the kind/meta The issue or PR is meta label Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/meta The issue or PR is meta
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Anki templates should sanitize inputs from document.
2 participants