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

FR: register for watchman trigger #3862

Merged
merged 1 commit into from
Jun 17, 2024
Merged

FR: register for watchman trigger #3862

merged 1 commit into from
Jun 17, 2024

Conversation

fowles
Copy link
Collaborator

@fowles fowles commented Jun 11, 2024

facebook/watchman#1221

I have not done anything in the checklist yet, just offering this for early review.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

Cargo.toml Outdated Show resolved Hide resolved
lib/src/fsmonitor.rs Outdated Show resolved Hide resolved
@fowles fowles force-pushed the main branch 5 times, most recently from 3cd0767 to 58f60e1 Compare June 15, 2024 16:01
@fowles
Copy link
Collaborator Author

fowles commented Jun 15, 2024

I have packed this into a single commit and internalized the extensions required to watchman, so I believe this is ready for final review. I still don't know how to unit test it.

@PhilipMetzger
Copy link
Collaborator

PhilipMetzger commented Jun 15, 2024

Small thing here, if you enable this it will probably break jj (op) undo as it we now start having very fine grained snapshots. I'd suggest to continue on #3700 if you want to keep a sane jj undo.

Optional: Change the commit title from feat: to watchman: as this project in general doesn't use conventional commits.

Copy link
Collaborator

@arxanas arxanas left a comment

Choose a reason for hiding this comment

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

Minor comments, overall it looks good to me! I think we should get sign-off from one other person before merging.

cli/src/config-schema.json Outdated Show resolved Hide resolved
lib/src/fsmonitor.rs Outdated Show resolved Hide resolved
lib/src/fsmonitor.rs Outdated Show resolved Hide resolved
cli/src/config-schema.json Outdated Show resolved Hide resolved
lib/src/fsmonitor.rs Outdated Show resolved Hide resolved
lib/src/fsmonitor.rs Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
@fowles fowles force-pushed the main branch 2 times, most recently from fb4718a to 89e044d Compare June 15, 2024 21:55
cli/src/config-schema.json Show resolved Hide resolved
lib/src/fsmonitor.rs Show resolved Hide resolved
lib/src/fsmonitor.rs Show resolved Hide resolved
lib/src/fsmonitor.rs Outdated Show resolved Hide resolved
lib/src/settings.rs Outdated Show resolved Hide resolved
@fowles fowles force-pushed the main branch 3 times, most recently from 120ea51 to 19c8e9b Compare June 16, 2024 17:27
- make an internal set of watchman extensions until the client api gets
  updates with triggers
- add a config option to enable using triggers in watchman

Co-authored-by: Waleed Khan <me@waleedkhan.name>
@fowles fowles merged commit 8aa71f5 into martinvonz:main Jun 17, 2024
17 checks passed
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.

4 participants