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

don't exclude README and CHANGELOG (and also Justfile) #13

Merged
merged 2 commits into from
Jun 22, 2024

Conversation

SillyFreak
Copy link
Contributor

@SillyFreak SillyFreak commented Jun 17, 2024

this removes the three items from exclude in typst.toml. The former two should be part of the published package; the latter should not even make it into the PR to universe and thus doesn't need to be excluded.

A possible alternative point of view is that the package script which uses .typstignore should not be assumed, and thus things like tests and Justfile should be put into exclude. Personally, I think that even when not using that script, these things don't belong into the PR and thus putting them in exclude sends the wrong signal, but it's not a strong preference.

Fixes #12

@tingerrr
Copy link
Member

My reasoning for putting them there was actually future proofing, since copying the files into the package repo is more or less a temporary solution. Typst will eventually have a proper publish workflow which would reasonably reuse this to exclude files while publishing.

@SillyFreak
Copy link
Contributor Author

SillyFreak commented Jun 17, 2024

exclude and what is submitted to Universe have (at least currently) different meanings though, see here: #12 (comment)

it can still make sense to defensively exclude a lot (but not README, at the very least) but there are files that should be excluded, yet definitely submitted to Universe

edit: wait, wasn't it James who chose the current list of ignored files?

@tingerrr
Copy link
Member

it can still make sense to defensively exclude a lot (but not README, at the very least) but there are files that should be excluded, yet definitely submitted to Universe

Yeah, I'm with you on that matter, I think the README should not be excluded.

edit: wait, wasn't it James who chose the current list of ignored files?

The repo structure was copied from valkryie, where I added most of the scaffolding like Justfile, tests workflow, and excludes from my packages.

You're definitely right about excluded and submitted being different things, which is a little annoying from a developer standpoint. I think most developers at this point just copy the whole repo verbatim, so a defensive exclude list could be beneficial.

@SillyFreak
Copy link
Contributor Author

Ok, then I'll add everything in typstignore to exclude as well

@jamesrswift
Copy link
Member

LGTM!

@SillyFreak
Copy link
Contributor Author

@tingerrr @Jamesxx would either of you merge this? I can't do it myself

@jamesrswift jamesrswift merged commit 41d6e49 into typst-community:main Jun 22, 2024
1 check passed
@SillyFreak SillyFreak deleted the clean-up-exclude branch June 22, 2024 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not exclude README.md
3 participants