-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
No prompt: checkout a file that changed #2803
Comments
This is looks like a lost of data, if I'm getting this right. This is not how Git behave. Also, not sure how is it different from one the #2801. |
There is no data loss - please check similar Git behavior (there is code example in #2498). Yes, it might be duplication with #2801 and that was mentioned in #2498 (comment). I assume these cases were brought separately based on code and it looks like dir/no-dir makes some difference in implementation. |
Ok, I think I see where that confusion comes from. It looks like you are suggesting way bigger changes than just replacing prompt with failure/warning. It seems there are two parts to it (all these tickets together - and I still don't know which one should I be using to keep discussion going). And that's where I was confused.
Since you connected these tickets with #2498 I think I was reading them like - "let's remove prompts w/o changing behavior of the dvc checkout". In this case there is a data loss. If these three tickets imply changes into dvc checkout semantics I would create a separate one to emphasize this and split the discussion. It's very confusing otherwise. And we need to start that by describing precisely how the new proposed checkout should behave in different circumstances - otherwise it's very hard to even review these tickets w/o understanding what exactly is suggested.
Even if it is the case I would try to generalize in a sense I described above:
|
The discussion is supposed to be going in the issue that we together created to summarise prompts stuff #2498. Some folks expressed their opinion there and these new issues were created to "execute" the summary discussion. I'm a bit surprised that after a month of silence in the summary issue we start the discussion again. Yes, it is a bit bigger issue than just replacing prompts by failings. I intended to give all theses implementation details to someone who will be implementing it without all the discussion we are making here. I don't think we need to waste time on further discussion and creating new issue - we already have on initial #2497, one summary #2498, 3 more detailed for the execution of the summary one #2801, #2802, #2803. If there is something to add - please share this in the summary issue #2498. |
Why don't we create a ticket and share "all theses implementation details" so that someone can start right away w/o some random siloed discussions? Why do we want to keep them ad-hoc and not transparent? So, that we can start necessary documentation changes as well.
I think #2498 is irrelevant to my concerns. To reiterate. There is a lot of confusion (at least for me) since we connected this with #2498 while actually it is about changing the checkout logic behavior in the first place. That's why I think the whole structure and emphasis of these tickets should be changed with a focus on that change in the first place and removing/replacing prompts only as a byproduct of that change. |
@shcheklein this is a good point about the logic change. I summarised all these implementations details in discussion #2498 since it affects all the cases (all 3 issues): #2498 (comment) |
These days checkout will depect a modified file (or multiple) and will just fail without doing anything unless |
See #2498
Some quotes:
It looks the same as the previous case (even though the implementation might be different). So, checkout should not raise any errors or prompts or even error messages.
I propose to avoid prompting and outputting any error messages or warning
The text was updated successfully, but these errors were encountered: