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

fix: use iarna/toml instead of toml #118

Merged
merged 10 commits into from
May 31, 2024

Conversation

ruben-arts
Copy link
Contributor

Using a better toml parser: https://shared.by.re-becca.org/misc/TOML-SPEC-SUPPORT.html

Fixes a known issue were the following table doesn't work but is valid toml:

[tool.pixi]
dependencies.boltons = "*"

@ruben-arts ruben-arts requested a review from pavelzw as a code owner May 31, 2024 05:49
@pavelzw
Copy link
Collaborator

pavelzw commented May 31, 2024

You need to add this library to the tsup config

@pavelzw
Copy link
Collaborator

pavelzw commented May 31, 2024

I think we should also wrap the readfile and parse toml in a try catch and do core.error if we catch something

const fileContent = readFileSync('pyproject.toml', 'utf-8')

The issue was a bit annoying to spot right now.

@pavelzw pavelzw added the bug Something isn't working label May 31, 2024
@ruben-arts
Copy link
Contributor Author

Is an core.error enough or should we add more info to it?

@pavelzw
Copy link
Collaborator

pavelzw commented May 31, 2024

I would add core.error("failed to parse pyproject.toml") and then throw the error again s.t. we see where this error was thrown. The previous error message (https://discord.com/channels/1082332781146800168/1082338253925003385/1245052133053829182) was not really helpful

All errors are catched and displayed in the outer most run action part

@ruben-arts
Copy link
Contributor Author

Yeah right! will do.

@ruben-arts
Copy link
Contributor Author

@ruben-arts
Copy link
Contributor Author

Or should we updated to pnpm 9?

And should I make a patch release?

@pavelzw
Copy link
Collaborator

pavelzw commented May 31, 2024

Let’s use pnpm 9 and a patch release 👍🏻

@pavelzw pavelzw merged commit ba3bb36 into prefix-dev:main May 31, 2024
126 of 129 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants