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

Manifests - to constrain, or not constrain #233

Closed
sdboyer opened this issue Feb 10, 2017 · 2 comments · Fixed by #1499
Closed

Manifests - to constrain, or not constrain #233

sdboyer opened this issue Feb 10, 2017 · 2 comments · Fixed by #1499
Labels

Comments

@sdboyer
Copy link
Member

sdboyer commented Feb 10, 2017

With the acceptance and closure of #213, we're clearly saying that we want our experiments to head in the direction of a hand-edited manifest. However, we're not so sure about a workflow in which it's "normal" not to list constraints in the manifest. So, this issue is to discuss that specific workflow.

Being that "discussing a workflow" is a bit interminable for a real issue, though, let's have the focus here be on the concrete implications:

  • What, exactly, should the docs say about this
  • What error/warning messages should be shown to the user, and when/by which commands, if there are imports without corresponding constraints in the manifest
@nathany
Copy link
Contributor

nathany commented Feb 13, 2017

My current take is that:

  • Adding an import (Go code) without a constraint (manifest) could just mean use the "best version" of that import with a smart default. No busy work of amending the manifest for the common case. Best version being:
    • Use "master" if the imported dependency currently has no tagged versions.
    • Prefer the "latest stable version" if tags are available.
    • Other constraints could result in a different version (as they do).
    • The best version could be redefined in the future as dep is enhanced (static analysis, avoid yanked versions due to CVEs, etc.).
  • Any constraint that is no longer imported would be an error similar to "imported and not used" (Go compiler), forcing the manifest file to be free of unused cruft.

One downside of this approach is that the manifest isn't a manifest, as it wouldn't provide a single place to view all the top-level dependencies. Instead it would just override default behaviour for dependencies at any given level. Unless the project in question needs pre-release versions or conflict resolution on a dependency, the manifest could be a blank file with a comment header.

IMO, this should work fine so long as the lock file and CLI work in concert to provide gradual upgrades to dependencies (v1 of foo doesn't jump to v2 without requesting the upgrade).

Nothing here prevents a library author from being more choosey. Saying library foo only works with x/net/http2: ^v1.0 and not ^v2.0. Those constraints tend to be added defensively because the expectation is that things will break. What this approach avoids is the manifest automatically being populated with those defensive constraints when running dep init.

(Of course I could be complete wrong about this idea, and it may take some playing around with it to see if it works well in practice).

@zevdg
Copy link

zevdg commented Feb 22, 2017

One downside of this approach is that the manifest isn't a manifest, as it wouldn't provide a single place to view all the top-level dependencies.

Given the import graph is the real datasource here. I don't see this as problem as much as an aesthetic choice. In terms of visibility/discovery, as long as dep or guru or some other tool can analyze the import graph and manifest and print out the top-level dependency information that people coming from other languages would expect in the manifest, then I don't see a real UX advantage to persisting this information to disk. If anything, the ability for it to get out of sync with the real import graph is a UX disadvantage.

This feels somewhat analogous to how people coming from other languages feel about implicit interfaces. The answer to "how do I tell what interfaces this type implements" is not reading text in a file, it's the guru tool. Similarly, the answer to "how do I see a list of my top-level dependencies" could be "dep status -v" or something like that.

edit: this feature is already requested in #257

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants