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(pkg): add location hash mismatch error #11007

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

rgrinberg
Copy link
Member

We use the lock directory as the location rather than pointing at the hash, as the user shouldn't really touch the hash.

Signed-off-by: Rudi Grinberg me@rgrinberg.com

@@ -41,6 +41,7 @@ We add the bar dependency to the test package

It fails as we have not regenerated the lock:
$ dune build
File "dune.lock", line 1, characters 0-0:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't it more confusing to have a pointer to the file with specific line and characters instead of just the instruction to run dune pkg lock? I would understand with just the filename (as a dir) but here, it might create the inverted effect and push the user to have a look at the directory. Wouldn't it be possible to have Directory "dune.lock" instead of File "dune.lock", line 1, characters 0-0?

Copy link
Member Author

Choose a reason for hiding this comment

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

Without the location, how does the user know which lock directory is out of date? Also, to display the error in the editor, some sort of location must be attached according to LSP.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with all of this. The part I find to be confusing is the mix of File and line whereas we are talking about a directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

I could pick the lock.dune file inside dune.lock.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You are right. It might be more understandable from a user perspective.

We use the lcok directory as the location rather than pointing at the
hash, as the user shouldn't really touch the hash.

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

<!-- ps-id: eee60b87-135e-4aa0-b63e-812c26bb44a2 -->
@rgrinberg rgrinberg force-pushed the ps/rr/fix_pkg___add_location_hash_mismatch_error branch from fddcaf7 to 6c3f488 Compare October 11, 2024 07:56
@rgrinberg rgrinberg merged commit 1becc6e into main Oct 11, 2024
27 checks passed
@rgrinberg rgrinberg deleted the ps/rr/fix_pkg___add_location_hash_mismatch_error branch October 11, 2024 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants