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

Use toml_edit for pretty tool receipt formatting #4637

Merged
merged 2 commits into from
Jun 29, 2024

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Jun 28, 2024

I started doing this because I wanted more control over the document but this ends up being useful for #4638

I worry about the complexity of this implementation for a largely internal document (unlike the lock file which is very user facing). I think #4638 might be better implemented by wrapping PathBuf. Then, in the future document could change to JSON with little effort. However, I think that we want tool definitions to be user-facing eventually so I'm unsure this is wasted effort or complexity.

@zanieb zanieb added the internal A refactor or improvement that is not user-facing label Jun 28, 2024
@zanieb zanieb marked this pull request as ready for review June 28, 2024 21:33
@zanieb zanieb added the preview Experimental behavior label Jun 28, 2024
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

I think it’s fine. We already have the dependency anyway.

As #4324 did for lock files, updates
the tool receipts to use portable paths. This resolves CI failures on
Windows in #4634 where the toml string literal type changes depending on
the platform.
@zanieb zanieb merged commit f71f738 into zb/tool-receipt-entry-points Jun 29, 2024
4 checks passed
@zanieb zanieb deleted the zb/tool-receipt-format branch June 29, 2024 03:34
zanieb added a commit that referenced this pull request Jun 29, 2024
We need this to power uninstallations! 

The latter two commits were reviewed in:

- #4637 
- #4638 

Note this is a breaking change for existing tool installations, but it's
in preview and very new. In the future, we'll need a clear upgrade path
for tool receipt changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal A refactor or improvement that is not user-facing preview Experimental behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants