-
Notifications
You must be signed in to change notification settings - Fork 451
Conversation
//Versioned changes should always be included | ||
if (resource.IsVersioned) { | ||
//Versioned changes should always be included (as long as they're not deletes) | ||
if (resource.IsVersioned && !resource.HasStatus(Status.DELETE)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the deletes are just included? does the checkin fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's the only file in the Included section, yes. Basically, it's a 'detected change' and the check-in fails with this:
[Checkin] No changes were matched by any arguments.
Presumably, if there are other files in that Included list, those would get checked in. Without this change, if the file is versioned and they delete it via the command prompt or explorer, it would appear in the Included list but not be checked in. With this change, it'll be excluded and when they move it to included, we'll run tf delete
on it.
} | ||
} catch (err) { | ||
//Provide a better error message if the file to be deleted isn't in the workspace (e.g., it's a new file) | ||
if (err.tfvcErrorCode && err.tfvcErrorCode === TfvcErrorCodes.FileNotInWorkspace) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we check ahead of time and just do a normal delete for them? I.e. so the user can ALWAYS call our delete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We certainly could but then we'd want to also do it for rename. I just followed the rename pattern (if the file isn't in the workspace, we won't run a "tfvc" command on it). I don't have passion either way although we could do this and see if we get any feedback. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Let's keep it this way and get user feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple comments but looks good!
In this PR, we've removed the automatic promoting of file deletions from candidate to pending changes. This should address #241.
Tested scenarios: