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

cli: make jj split more explicit about filesets #5097

Merged
merged 1 commit into from
Dec 15, 2024
Merged

Conversation

fowles
Copy link
Collaborator

@fowles fowles commented Dec 13, 2024

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

Copy link
Collaborator

@PhilipMetzger PhilipMetzger left a comment

Choose a reason for hiding this comment

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

While I really like how simple this looks, we've only had inconclusive discussions around multi-splitting in Discord. This also resurfaces #3879/#3549 (comment) which hasn't gotten any valuable traction.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@fowles fowles force-pushed the split branch 2 times, most recently from 4c36266 to 4205ec0 Compare December 15, 2024 00:17
@fowles fowles changed the title cli: add --second to split command cli: make jj split more explicit about filesets Dec 15, 2024
PhilipMetzger
PhilipMetzger previously approved these changes Dec 15, 2024
Copy link
Collaborator

@PhilipMetzger PhilipMetzger left a comment

Choose a reason for hiding this comment

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

LG

CHANGELOG.md Outdated Show resolved Hide resolved
@PhilipMetzger PhilipMetzger dismissed their stale review December 15, 2024 18:14

We need to know how much the feature is actually needed as by Martin.

@arxanas
Copy link
Collaborator

arxanas commented Dec 15, 2024

This seems like a great change to me. I was also personally unaware that jj split, etc. accepted filesets rather than just paths. We should update all the other places that accept filesets at some point as well. (I'll leave @martinvonz to give the final sign-off since this is set to auto-merge.)

In principle: I suppose a user might check the help text and be a little put off or scared by talk of "filesets" instead of just "files" or "paths". But I'm optimistic they'll have seen enough example usage to just ignore what they don't understand and pass paths normally, and treat "filesets" as a black box abstraction if necessary.

Copy link
Owner

@martinvonz martinvonz 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 every command that uses parse_file_patterns() could use the same update. I can try to follow up with that.

@fowles fowles merged commit c82483b into martinvonz:main Dec 15, 2024
18 checks passed
@yuja
Copy link
Collaborator

yuja commented Dec 16, 2024

I think every command that uses parse_file_patterns() could use the same update. I can try to follow up with that.

Maybe we should update -r <REVISION> to -r <REVSET> for consistency? (I don't think we'll need to rename internal variables.)

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.

6 participants