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

ensure: add tests and behavior for when a dep is unused #108

Closed
wants to merge 2 commits into from
Closed

ensure: add tests and behavior for when a dep is unused #108

wants to merge 2 commits into from

Conversation

jessfraz
Copy link
Contributor

@jessfraz jessfraz commented Jan 18, 2017

behaves in the following way:

$ ./hoard ensure github.com/jessfraz/[email protected]
WARNING: github.com/jessfraz/libyubikey was requested but is not imported in your code.

$ ./hoard require github.com/jessfraz/libyubikey

part of #36

Copy link
Member

@sdboyer sdboyer left a comment

Choose a reason for hiding this comment

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

so a -require flag is gonna be a bit different because gps treats requires (and ignores) at the package-level, not the project level.


if !found {
fmt.Fprintf(os.Stdout, `WARNING: %s was requested but is not imported in your code.
Run again with --require to ensure it is vendored and added to the lock.
Copy link
Member

Choose a reason for hiding this comment

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

nit: one dash

@@ -153,6 +157,44 @@ func (cmd *ensureCommand) Run(args []string) error {
}
}

// add all the required packages to required in the manifest
for _, arg := range cmd.requires {
pc, err := getProjectConstraint(arg, sm)
Copy link
Member

Choose a reason for hiding this comment

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

because requires are pkg-level rather than project-level, i wonder if it might be better to not allow the user to specify a constraint at the same time as they specify a require (or an ignore)

Copy link
Contributor Author

@jessfraz jessfraz Jan 18, 2017

Choose a reason for hiding this comment

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

Hmmm idk because then they would have to put it twice to ensure we could just say you get the project at the constraint but you are requiring that it doesn't disappear since you aren't using it

@sdboyer sdboyer mentioned this pull request Jan 27, 2017
@sdboyer
Copy link
Member

sdboyer commented Feb 22, 2017

I'm gonna close this out, since we're moving towards a different vision of ensure now.

@sdboyer sdboyer closed this Feb 22, 2017
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.

3 participants