Skip to content

Conversation

whereistejas
Copy link

@whereistejas whereistejas commented Oct 10, 2025

Closes #6556.

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/updated tests to cover my changes

@whereistejas whereistejas requested a review from a team as a code owner October 10, 2025 14:07
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

The following commits do not follow our format for subject lines:

  • a98a670: Use print_unmatched_explicit_paths in cmd_status.
  • 14e7bbd: Update CHANGELOG

Commits should have a subject line following the format <topic>: <description>. Please review the commit guidelines for more information.

@whereistejas whereistejas changed the title jj status should throw an error when the fileset does not exist jj status should throw a warning when the fileset does not exist Oct 10, 2025
@whereistejas whereistejas force-pushed the warn-when-fileset-is-missing branch from 14e7bbd to 84fb867 Compare October 10, 2025 14:11
@github-actions github-actions bot dismissed their stale review October 10, 2025 14:11

All commits are now correctly formatted. Thank you for your contribution!

@whereistejas whereistejas force-pushed the warn-when-fileset-is-missing branch from 84fb867 to bea6920 Compare October 10, 2025 14:15
CHANGELOG.md Outdated
Comment on lines 28 to 29
* `jj status` now throws a warning when the fileset does not exist.

Copy link
Member

Choose a reason for hiding this comment

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

nit: Please squash this change into the parent commit.

I'm not sure we even need to mention it in the changelog. Users will notice the new warning when they run into it anyway. I can see some small value in having it there e.g. if a user is wondering why they're not seeing the warning when they're on an older version than their friend who is seeing the warning. Then they can check the changelog to see why. If we do add the changelog entry, it would be nice if it was just one entry for all remaining commands (see my other comment), so we don't end up with 10 different entries as we add the warning in new places.

@whereistejas whereistejas force-pushed the warn-when-fileset-is-missing branch 3 times, most recently from fb17003 to d6a7b86 Compare October 13, 2025 16:30
@whereistejas whereistejas changed the title jj status should throw a warning when the fileset does not exist jj commands should throw a warning when the fileset does not exist Oct 13, 2025
@whereistejas whereistejas force-pushed the warn-when-fileset-is-missing branch 3 times, most recently from 0eeeaac to 0c2a608 Compare October 13, 2025 17:30
@whereistejas whereistejas force-pushed the warn-when-fileset-is-missing branch from 0c2a608 to 38cfcd1 Compare October 21, 2025 12:33
@whereistejas whereistejas force-pushed the warn-when-fileset-is-missing branch from 38cfcd1 to 5e3fd26 Compare October 21, 2025 15:15
@whereistejas whereistejas force-pushed the warn-when-fileset-is-missing branch from 5e3fd26 to 55ab0d8 Compare October 21, 2025 15:46
@glehmann
Copy link
Contributor

IMO, you should merge the revision that changes the command and the revision that adds the test for each command. It would be easier to review.
And it's a bit strange, given how the revisions are organized, that the number of revisions in the PR is odd :)

@whereistejas
Copy link
Author

whereistejas commented Oct 22, 2025

IMO, you should merge the revision that changes the command and the revision that adds the test for each command. It would be easier to review.

I want to make sure that the warning is shown in a consistent manner across all commands. That's why the commands and tests are one after another. I would be happy to split the PR in whatever way is more convenient for the reviewer when ready :)

And it's a bit strange, given how the revisions are organized, that the number of revisions in the PR is odd :)

I didn't have to add tests for all the commands. Some of them already had tests capturing this behaviour, for example, split.

@martinvonz
Copy link
Member

nit: Could you squash the tests into the commit they're testing? See https://jj-vcs.github.io/jj/latest/contributing/#commit-guidelines

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.

FR: jj status should throw an error when the fileset does not exist

3 participants