Add "status" and "baseline" commands, add a "--safe" sync flag#436
Add "status" and "baseline" commands, add a "--safe" sync flag#436cdevienne wants to merge 10 commits intocarvel-dev:developfrom
Conversation
For git & mercurial fully cloned directories, the status command returns a summary of local changes. Signed-off-by: Christophe de Vienne <christophe.devienne@orus.io>
Signed-off-by: Christophe de Vienne <christophe.devienne@orus.io>
Signed-off-by: Christophe de Vienne <christophe.devienne@orus.io>
Signed-off-by: Christophe de Vienne <christophe.devienne@orus.io>
This command updates the git & hg 'ref' field based on the actual checked-out commit of the corresponding content paths Signed-off-by: Christophe de Vienne <christophe.devienne@orus.io>
cdevienne
force-pushed
the
cmd-status
branch
from
da28d61 to
4c166f1
Compare
|
I did a few changes to make the linter happier, but if I take everything in consideration I feel the code gets less readable. The thing is that I realized that most of the vendir code does not respect these settings: so, should I strictly comply to what the linter says ? or would it make sense to change 'revive' settings ? |
Apply some of the advice of 'revive'. Signed-off-by: Christophe de Vienne <christophe.devienne@orus.io>
cdevienne
force-pushed
the
cmd-status
branch
from
c838b0b to
2e81a14
Compare
Signed-off-by: Christophe de Vienne <christophe.devienne@orus.io>
…ironment variable Signed-off-by: Christophe de Vienne <christophe.devienne@orus.io>
When the ref is a branch and still matches, it should not be replaced with sha. Signed-off-by: Christophe de Vienne <christophe.devienne@orus.io>
joaopapereira
left a comment
joaopapereira
left a comment
There was a problem hiding this comment.
Here are my initials impressions:
- The status looks pretty straightforward, despite the fact that I am not a fan of the tabular output I am not opposed.
- I would prefer the baseline and status to be in different PRs because they are quite different features, despite the fact that they reuse some internal functions
- In my opinion the baseline command should not be successful if there are uncommitted changes in a folder because we should not save none reproducible behaviors.
- Looks like we are not updating the lock file when we do the baseline command, that does not sound ok.
- in terms of the linter I am not sure why it is so mad lately but I am ok with not addressing the issues we see in the status (for more context here, we added the linter way later in the project and decided to fix the linter issues as we did changes in the code instead of doing a full sweep of the full code trying to fix everything)
Let me know if you have any question about my review. Also we need some tests to make sure this is working in the future if there is any change to this code.
| status.Ref.Tags = slices.DeleteFunc( | ||
| status.Ref.Tags, func(t string) bool { return t == "tip" }) | ||
| } | ||
| if branch != emptyString { |
There was a problem hiding this comment.
Can this piece of code not be just
status.Ref.Others = append(status.Ref.Others, branch, topic, bookmarks)?
Because if it is an empty string it will append "" which should not be a problem
| TargetRef string | ||
| Ref CompleteReference | ||
| UncommitedChanges []string | ||
| LocalCsets []string |
There was a problem hiding this comment.
With this name I am not sure what will be present in this variable.
|
|
||
| newRefs := make(map[string]string) | ||
|
|
||
| for _, status := range statusMap { |
There was a problem hiding this comment.
If you have uncommitted changes in a particular directory I think you should not allow rebasing because you are setting the state to something that cannot be reproducible.
| return f.Close() | ||
| } | ||
|
|
||
| func updateRefs(fname string, newRefs map[string]string) error { |
There was a problem hiding this comment.
feels like we should use the same mechanism here to generate the lock and config file that we use in the sync commands. This "snipping" changes in the configuration can make future changes more complicated.
2 participants
This PR implements my ideas for addressing #388.
This is the initial implementation of these features, and we are starting to use them internally.
Even though I expect some changes to be done based on our usage, a first review of the general approach would be nice, so I can take your feedback into consideration as soon as possible.
The main point I still need to address is how to enable the safe mode by default for a user or a project.