Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Implement missing flag for dep status #1870

Conversation

tinnefeld
Copy link
Contributor

What does this do / why do we need it?

Comply with command spec by making dep status -missing
list the dependencies that are missing from the lock file
but are used in the project.

If there are no dependencies missing it returns "No missing
dependencies found." If there are errors preventing the
execution of dep status, the error msg will be displayed instead.

What should your reviewer look out for in this PR?

Obviously, this will be superseded by the new status run mode currently WIP in [1]. However, it brings compatibility with the spec [2] / gets rid of "not implemented" in the meantime.

[1] #613
[2] https://docs.google.com/document/d/1Qwa3jDKDh45t5qAGjRpeu3ZsAbkukIYDDmYJdMxH2uY/edit?usp=sharing

Which issue(s) does this PR fix?

fixes #1382

Comply with command spec by making dep status -missing
list the dependencies that are missing from the lock file
but are used in the project.

If there are no dependencies missing it returns "No missing
dependencies found." If there are errors preventing the
execution of dep status, the error msg will be displayed instead.
@darkowlzz
Copy link
Collaborator

Thanks for creating this PR.

Looks like there is some misunderstanding in what we want from the -missing flag. As per your changes, even when status is run in missing mode, the whole runStatusAll() would be run. That is not what we would like to do. As shown in #613 , we go through the external package list and lock project list and pick out the projects that don't exist in the lock file.

#613 was just an experiment but #1382 is for a proper implementation of the -missing flag.

Can you refer to runStatusMissing() implementation in #613 and add that to this PR? That is more in line with what is expected for #1382 .

Hope this helps. Feel free to ask if there's any confusion.

Comply with command spec by making dep status -missing
list the dependencies that are missing from the lock file
but are used in the project. Issue golang#1382.
@tinnefeld tinnefeld requested a review from sdboyer as a code owner May 28, 2018 19:42
@tinnefeld
Copy link
Contributor Author

Thank you for the detailed explanation, feedback on the updated PR is appreciated.

Also, since #613 is a bit older, there is now some "more advanced" stuff in runStatusAll() such as the outputter and the ability to list missing packages per project. A short clarification if the -missing flag should deliberately not adopt them to keep it simple or if they should be picked up eventually would be nice.

Copy link
Collaborator

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

Also, since #613 is a bit older, there is now some "more advanced" stuff in runStatusAll() such as the outputter and the ability to list missing packages per project. A short clarification if the -missing flag should deliberately not adopt them to keep it simple or if they should be picked up eventually would be nice.

No, we don't need that. Missing is project specific, no need to list missing packages per project.

This looks good to me. But as in the spec doc, we need to support output formats, template (-f) and json (-json) format. Can you add those as well? If not, we can add them in a follow-up PR.

@@ -0,0 +1,13 @@
// Copyright 2017 The Go Authors. All rights reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

2017 -> 2018

},
wantStatus: "Missing dependencies (not present in lock):\n" +
" github.com/boltdb/bolt\n" +
" github.com/sdboyer/dep-test\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use back tick (`) as well if you would like to avoid appending with + and \n.

Comply with command spec by making dep status -missing
list the dependencies that are missing from the lock file
but are used in the project. Issue golang#1382.
@tinnefeld tinnefeld force-pushed the issue-1382-missing-flag-for-dep-status branch from 6ea763c to 391e768 Compare July 8, 2018 01:01
@tinnefeld
Copy link
Contributor Author

tinnefeld commented Jul 8, 2018

Thank you for the feedback, I incorporated it and added corresponding tests. Sorry for the delay.

@sdboyer
Copy link
Member

sdboyer commented Jul 11, 2018

i'm gonna keep this on hold for after v0.5.0 - #1912, and now #1932, will likely change the way we want to do this a fair bit.

@lubo
Copy link

lubo commented Jan 15, 2019

@sdboyer v0.5.0 is out. Just sayin'…

@mvdan
Copy link
Member

mvdan commented Sep 4, 2020

Dep was officially deprecated earlier this year, and the proposal to archive this repository was accepted. As such, I'm closing outstanding issues before archiving the repository. For any further comments, please use the proposal thread on the Go issue tracker. Thanks!

@mvdan mvdan closed this Sep 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement -missing flag for dep status
6 participants