Skip to content

Conversation

isuffix
Copy link
Contributor

@isuffix isuffix commented Aug 18, 2025

I originally just added these with a From impl like below. But I figure that just extending the enums is prefered. Let me know if the From impl would be better instead.

impl From<WorkingCopyStateError> for SnapshotError {
    fn from(WorkingCopyStateError { message, err }: WorkingCopyStateError) -> Self {
        Self::Other { message, err }
    }
}

I also have two TODO comments where I wasn't sure whether converting the error type automatically would be an improvement. I would appreciate feedback on those.

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

@isuffix isuffix requested a review from a team as a code owner August 18, 2025 02:16
@isuffix isuffix force-pushed the push-tsyxmzqxyppy branch from 94a3c26 to 2396e7e Compare August 18, 2025 02:29
@isuffix isuffix force-pushed the push-tsyxmzqxyppy branch from 2396e7e to de587a6 Compare August 18, 2025 13:52
@isuffix isuffix enabled auto-merge August 18, 2025 13:53
@isuffix
Copy link
Contributor Author

isuffix commented Aug 24, 2025

Is there anything else I should do for this?

@isuffix isuffix force-pushed the push-tsyxmzqxyppy branch from de587a6 to 6f93a37 Compare August 24, 2025 23:18
@isuffix isuffix requested a review from martinvonz August 27, 2025 02:15
@isuffix isuffix force-pushed the push-tsyxmzqxyppy branch 3 times, most recently from d3c90e2 to 2d0437b Compare September 3, 2025 01:56
@isuffix isuffix requested review from yuja and removed request for yuja September 3, 2025 02:10
Note that the actual error message is almost always the same. They all end up
with the message "Failed to read working copy state" in `tree_state`. So the
original code adding another equivalent message is unnecessary.
@isuffix isuffix disabled auto-merge September 3, 2025 13:18
@isuffix isuffix enabled auto-merge September 3, 2025 13:19
@isuffix isuffix added this pull request to the merge queue Sep 3, 2025
Merged via the queue into jj-vcs:main with commit ee6ae06 Sep 3, 2025
29 checks passed
@isuffix isuffix deleted the push-tsyxmzqxyppy branch September 3, 2025 13:36
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.

2 participants