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

Expand example Gopkg.toml text; always add on init #462

Merged
merged 5 commits into from Apr 28, 2017

Conversation

Projects
None yet
5 participants
@sdboyer
Copy link
Member

sdboyer commented Apr 26, 2017

This expands the example text we add to Gopkg.toml to cover all valid values for the file.

It also changes the behavior of dep init to write out the example data unconditionally to the bottom of Gopkg.toml, rather than doing so only if nothing was to populate the manifest.

I'm not sure this latter part is the best idea, but being that there will be precious few situations under which we'd have an empty manifest (once init changes are done, #277), it seems worth being more aggressive about writing out these examples.

We could also add a flag to init that allows the user to suppress writing out the examples.

@googlebot googlebot added the cla: yes label Apr 26, 2017

@sdboyer sdboyer force-pushed the sdboyer:expand-examples branch from f3da373 to ad41001 Apr 26, 2017

@zellyn

zellyn approved these changes Apr 26, 2017

Copy link
Contributor

zellyn left a comment

lgtm

# revision = "abc123"

## Overrides have the same structure as [[dependencies]], but supercede all

This comment has been minimized.

@zellyn

zellyn Apr 26, 2017

Contributor

supersede

This comment has been minimized.

@sdboyer

sdboyer Apr 26, 2017

Member

hah, no kidding 😛

##
## Overrides are a sledgehammer, and should be used only as a last resort.
# [[overrides]]
## Required: the root import path of the project being constrained

This comment has been minimized.

@zellyn

zellyn Apr 26, 2017

Contributor

Perhaps just note that they have the same format, rather than repeating everything?

This comment has been minimized.

@sdboyer

sdboyer Apr 26, 2017

Member

Yeah, that'll help cut down on length, 👍

## can be used to require "main" packages.
# required = ["github.com/user/thing/cmd/thing"]

## "ignored" lists a set of packages (not projects) that are ignored when

This comment has been minimized.

@zellyn

zellyn Apr 26, 2017

Contributor

s/ignored when/ignored completely when/

## Optional: an alternate location (URL or import path) for the project's source
# source = "https://github.com/myfork/package.git"
## Optional, but recommended: the version constraint to enforce for the project.
## Only one of "branch", "version" or "revision" can be specified.
# version = "1.0.0"

This comment has been minimized.

@fortytw2

fortytw2 Apr 26, 2017

is this a constraint string like ">=0.4.0, <1.0.0" or a specific version it has to be? Might be helpful to briefly describe these in here, or add a second line to make this behavior clear

This comment has been minimized.

@sdboyer

sdboyer Apr 26, 2017

Member

It can be either (though note that when #225 goes in, 1.0.0 will be interpreted as ^1.0.0). It can also specify an entirely non-semver tag for a literal match.

We need a more formal writeup of all of these rules. The examples here should really just be terse, and point to said writeup.

@sdboyer sdboyer force-pushed the sdboyer:expand-examples branch from 3be598e to 1b707ca Apr 28, 2017

@sdboyer sdboyer force-pushed the sdboyer:expand-examples branch from 1b707ca to f4079c9 Apr 28, 2017

Put the examples at the top instead
Injecting the comments at the bottom is suboptimal, as it means that
additional dependencies injected by later `dep ensure` calls (under
the new spec) will be separated from the original `init` set.

This seems slightly annoying now, but once we add a flag to allow
bypassing example injection, it'll be less of a problem.
const exampleTOML = `
## EXAMPLES & DOCS - safe to delete!
#
## Dependencies define constraints on how dependent projects should be

This comment has been minimized.

@adg

adg Apr 28, 2017

Contributor

Dependencies define constraints on dependent projects. The bit about Gopkg.lock is a bit jargony and I don't think it adds anything.

This comment has been minimized.

@sdboyer

sdboyer Apr 28, 2017

Member

yeah that's fair, i was trying to make more of a distinction than we really needed

const exampleToml = `
# Example:
const exampleTOML = `
## EXAMPLES & DOCS - safe to delete!

This comment has been minimized.

@adg

adg Apr 28, 2017

Contributor

Example (these lines may be deleted)

# source = "https://github.com/myfork/package.git"
## Optional, but recommended: the version constraint to enforce for the project.

This comment has been minimized.

@adg

adg Apr 28, 2017

Contributor

Put the optional but recommended one above the optional one?

This comment has been minimized.

@sdboyer
# branch = "master"
# name = "github.com/vendor/package"
# Note: revision will depend on your repository type, i.e git, svc, bzr etc...
## Note: revision will depend on your repository type; git and hg have SHA1s,

This comment has been minimized.

@adg

adg Apr 28, 2017

Contributor

This note is unnecessary IMO

This comment has been minimized.

@sdboyer

sdboyer Apr 28, 2017

Member

Dropped it

#
## Dependencies define constraints on how dependent projects should be
## incorporated into Gopkg.lock. They are respected by dep whether
## this project is the current project, or if it's a dependency.
# [[dependencies]]

This comment has been minimized.

@adg

adg Apr 28, 2017

Contributor

Perhaps each line should precede its documentation. It might be clearer that way.

# [[dependencies]]
## The dependencies section defines constraints ...
# name = "..."
## The required name field specifies the root...
# revision = "abc123"
# version = "1.0.0"
#
## [[overrides]] follow the exact same structure as [[dependencies]], but supercede

This comment has been minimized.

@adg

adg Apr 28, 2017

Contributor
# [[overrides]]

??

This comment has been minimized.

@sdboyer

sdboyer Apr 28, 2017

Member

yeah, restructuring this. I dropped the section overall because it's duplicative of [[dependencies]], but i'm re-adding it because the comment text should actually be different.

@sdboyer

This comment has been minimized.

Copy link
Member

sdboyer commented Apr 28, 2017

Reorganized and fleshed out the text a bit more. Had to put the required and ignored elements up top, or they're actually being grouped in with the previous table array elem.

const exampleToml = `
# Example:
const exampleTOML = `
## EXAMPLE (these lines may be deleted)

This comment has been minimized.

@adg

adg Apr 28, 2017

Contributor

Can we please not shout? :-)

## project. Use it when your project needs a package it doesn't explicitly import -
## including "main" packages.
# required = ["github.com/user/thing/cmd/thing"]
#

This comment has been minimized.

@adg

adg Apr 28, 2017

Contributor

single blank please

This comment has been minimized.

@sdboyer

sdboyer Apr 28, 2017

Member

Heh, I introduced these to try to visually delineate between the groups of items, but now that their ordering is actually correct, can just rely on...yknow, TOML's rules 😄

const exampleTOML = `
## EXAMPLE (these lines may be deleted)
#
## "required" lists a set of packages (not projects) that must be included in

This comment has been minimized.

@adg

adg Apr 28, 2017

Contributor

What I was suggesting in the previous round of comments was that the docs should be after the thing they document, which I know is different to how it works in Go code, but in this case we're looking at a config example so I think it makes some kind of sense.

This comment has been minimized.

@sdboyer

sdboyer Apr 28, 2017

Member

I can see how that makes sense with the [[dependencies]], where it looks more or less like a section header. But with I think it'd be more confusing to do that on individual values. I can't think of a commented config file I've seen that puts descriptive docs below the values they describe.

## Dependencies define constraints on dependent projects. They are respected by
## dep whether coming from the Gopkg.toml of the current project or a dependency.
#
## Required: the root import path of the project being constrained.

This comment has been minimized.

@adg

adg Apr 28, 2017

Contributor

It's easier to read if this is just an actual sentence.

The required name field specifies the root import path...

This comment has been minimized.

@sdboyer

sdboyer Apr 28, 2017

Member

Fair point - I'll see if I can rework the others, then. Mostly, I care about consistency

@sdboyer sdboyer force-pushed the sdboyer:expand-examples branch from bec2f35 to f861939 Apr 28, 2017

@adg

adg approved these changes Apr 28, 2017

@sdboyer sdboyer merged commit 11b52a3 into golang:master Apr 28, 2017

3 checks passed

cla/google All necessary CLAs are signed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sdboyer

This comment has been minimized.

Copy link
Member

sdboyer commented Apr 28, 2017

@adg @fortytw2 @zellyn thanks for review!

@carolynvs carolynvs referenced this pull request May 5, 2017

Merged

Remove unused function #524

ibrasho pushed a commit to ibrasho-forks/dep that referenced this pull request May 10, 2017

Merge pull request golang#462 from sdboyer/expand-examples
Expand example Gopkg.toml text; always add on init
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment