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

hagaki:0.1.0 #1505

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

hagaki:0.1.0 #1505

wants to merge 4 commits into from

Conversation

34j
Copy link

@34j 34j commented Jan 2, 2025

I am submitting

  • a new package
  • an update for a package

Allows batch printing address labels on Japanese Hagaki paper (from a CSV file).

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

@typst-package-check typst-package-check bot added the new A new package submission. label Jan 2, 2025
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 submitting this package. In addition to the comments below, could you please change the name to something less canonical? Ideally, package names should be unique enough to allow for alternatives without one of them being seen as "the" package for that purpose. If you lack inspiration, you can simply add an adjective to the current name.

Comment on lines +161 to +166
/// - path (string): The path to the CSV file, e.g., "jyusyoroku.csv".
/// - lastNameColumn (integer): The column number for the last name, e.g., 0.
/// - firstNameColumn (integer): The column number for the first name, e.g., 1.
/// - postCodeColumn (integer): The column number for the post code, e.g., 2.
/// - addressColumn (integer): The column number for the address, e.g., 3.
/// - myLine (number): The line number for the sender, e.g., 1.
Copy link
Member

Choose a reason for hiding this comment

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

The names of the parameters here seem outdated.

my-line: 1,
debug: false,
) = {
let lines = csv(path)
Copy link
Member

Choose a reason for hiding this comment

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

This will probably not work as expected, since you can only access files from your package if you make the csv call here. Instead, you should ask the user of the package to pass a call to csv(path) instead of just path as a parameter.

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