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 comodity name string if metadata is not defined #64

Conversation

Evernight
Copy link
Contributor

It seems that the new version expects name always be present in the commodity metadata and throws an unhandled error if it's not.
This will use commodity string itself as a backup (probably it wouldn't break anything?)
The new version looks amazing, by the way, thanks!

@Evernight Evernight force-pushed the commodity-string-instead-of-metadata branch from 3ced616 to 60f264f Compare February 6, 2025 12:36
@Evernight
Copy link
Contributor Author

A couple of notes on running dev environment locally:

  1. Was getting
error: Failed to parse `uv.lock`
  Caused by: TOML parse error at line 398, column 1
    |
398 | [[package]]
    | ^^^^^^^^^^^
missing field `version`

on running most make commands. Fixed temporarily by specifying version in the related section.

  1. After that make test fails with a lot of changes while comparing resulting html that seem to be not related to the commit (part of it seems related to some localization settings, e.g. renders $ as US$ locally for some reason). My guess is that something from the dependencies changed or different versions are fetched on my system.

@andreasgerstmayr
Copy link
Owner

Thanks for the PR!

A couple of notes on running dev environment locally:

1. Was getting
error: Failed to parse `uv.lock`
  Caused by: TOML parse error at line 398, column 1
    |
398 | [[package]]
    | ^^^^^^^^^^^
missing field `version`

on running most make commands. Fixed temporarily by specifying version in the related section.

Is this with the latest uv version?

2. After that `make test` fails with a lot of changes while comparing resulting html that seem to be not related to the commit (part of it seems related to some localization settings, e.g. renders `$` as `US$` locally for some reason). My guess is that something from the dependencies changed or different versions are fetched on my system.

Yep, unfortunately the e2e tests are influenced by the system (font rendering, locale, etc.). I always run them in the Dev Container, to have a consistent environment.

But I understand that Dev Containers are not everyones thing, so I just added a new Makefile target container-test which runs the tests in a container and should give reproducible results, independent of the host environment.

@andreasgerstmayr andreasgerstmayr merged commit 84c81b0 into andreasgerstmayr:main Feb 6, 2025
1 check passed
@Evernight
Copy link
Contributor Author

Is this with the latest uv version?

Turned out it was indeed not the latest (0.5.13 vs 0.5.29), pip install --upgrade uv resolved it. I thought I installed it quite recently but also it seems uv's development is going at quite a pace :).

Thank you!

@Evernight Evernight deleted the commodity-string-instead-of-metadata branch February 7, 2025 13:02
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.

2 participants