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

watchman: switch from a private extension to the newly release 0.9.0 #3913

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

fowles
Copy link
Collaborator

@fowles fowles commented Jun 18, 2024

No description provided.

Copy link
Collaborator

@thoughtpolice thoughtpolice left a comment

Choose a reason for hiding this comment

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

Note that this will break both:

  • The Nix build (which will require a change to cargoLock.outputHashes, so it's fixable at least).[1] But more importantly,
  • Publishing to crates.io, because it does not allow a crate to have unpublished dependencies (unfixable).

So, this will probably have to go on hold/draft until a new watchman_client is released anyway, I'm afraid, which is why I'm giving it a -1 (really, the only reason). Otherwise it LGTM. You may already be aware of that of course, but I'm writing it here for anyone else coming by. :)

Is there anything particularly wrong with using the current code path? Or is the main advantage just being able to delete some code? We can have a TODO or bug assigned for that, at least.

[1] And even though it's fixable, I'd really prefer we didn't do it anyway, because in practice, at least IME, if people ever need to readjust it or fix it again, they may have no idea how to do so if they aren't familiar with Nix and have it installed. I think that's a pretty poor experience for most of us (spoken as the main developer using it — in general, I'm quite happy that our Nix builds have very little overhead for the main workflows.)

@fowles
Copy link
Collaborator Author

fowles commented Jun 19, 2024

@thoughtpolice Given that the client had not had.a new version for 2 years, I was worried that they wouldn't spin one soon. But I filled an issue and they released 0.9.0 earlier today. So I have updated this to work on the released version. :)

@fowles fowles changed the title Switch from a private extension to an unreleased watchman_client watchman: switch from a private extension to the newly release 0.9.0 Jun 19, 2024
@fowles fowles enabled auto-merge (rebase) June 19, 2024 03:13
@fowles fowles dismissed thoughtpolice’s stale review June 19, 2024 03:14

The requested change has been made.

@fowles fowles merged commit 95fbf9f into martinvonz:main Jun 19, 2024
17 checks passed
@fowles fowles deleted the watchman branch June 19, 2024 03:15
@thoughtpolice
Copy link
Collaborator

Given that the client had not had.a new version for 2 years, I was worried that they wouldn't spin one soon.

Makes sense, thanks!

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.

3 participants