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

wenyuan-campaign:0.1.0 #1328

Merged
merged 8 commits into from
Nov 28, 2024
Merged

wenyuan-campaign:0.1.0 #1328

merged 8 commits into from
Nov 28, 2024

Conversation

yanwenywan
Copy link
Contributor

I am submitting

  • a new package
  • an update for a package

Description: This package recreates the LaTeX DnD 5e-style campaign document for Typst, for all game masters who want to write (or publish!) their campaign documents in a format like published campaigns, and for reasons prefer to use typst or do not want to go through the trouble of manually installing a LaTeX package, dealing with long compile times, etc.

I have read and followed the submission guidelines and, in particular, I

  • selected a name that isn't the most obvious or canonical name for what the package does
  • added a typst.toml file with all required keys
  • added a README.md with documentation for my package
  • have chosen a license and added a LICENSE file or linked one in my README.md
  • tested my package locally on my system and it worked
  • excluded PDFs or README images, if any, but not the LICENSE
  • ensured that my package is licensed such that users can use and distribute the contents of its template directory without restriction, after modifying them through normal use.

@typst-package-check typst-package-check bot added the new A new package submission. label Nov 25, 2024
@yanwenywan
Copy link
Contributor Author

Not quite sure why the package check is bringing up "This PR does too many things", please advise if there's any issues, thanks

Copy link
Member

@elegaanz elegaanz left a comment

Choose a reason for hiding this comment

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

Thank you for the nice template!

Sorry about the CI bug, it is a known. I did a manual review instead.

For info, it is more common in the Typst ecosystem to use kebab-case names for functions and their arguments, so you may want to change that (but you can also keep camelCase if you prefer).

@yanwenywan
Copy link
Contributor Author

Thanks for you feedback. It's no problem to change the names, I don't think I ever realised Typst allows kebab-case until recently lol

@typst-package-check typst-package-check bot changed the title dndcampaign:0.1.0 wenyuan-campaign:0.1.0 Nov 26, 2024
@yanwenywan
Copy link
Contributor Author

yanwenywan commented Nov 26, 2024

Okay, it's still unhappy about a camelCase name, but that's not really part of the intended public interface 😅 everything that's meant to be used by a user should be now kebab case

Also, sorry about the few last straggler bugs and features that need to be added in before merge

@elegaanz
Copy link
Member

Thank you for renaming the package and fixing the issues.

FYI, you can silence warnings about kebab-case by prefixing names with a _. The item would still be accessible, be it is clearer that it is supposed to be private. If you want it to not be public at all, the correct solution is to have an entrypoint file that only imports items that should be public from other files. I'm not suggesting you to do that here, just giving you the information so that you have it next time.

Also, sorry about the few last straggler bugs and features that need to be added in before merge

Please tell me when you're ready for another review then :)

@yanwenywan
Copy link
Contributor Author

Thanks for your advice. I've gone through the package once more and ported one of my own small campaigns to typst, I can't see any more problems so it's probably good to release to the wild

Ready for review whenever you are.

@elegaanz
Copy link
Member

Great, everything looks good to me, thanks!

@elegaanz elegaanz merged commit 1627c5b into typst:main Nov 28, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new A new package submission.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants