Skip to content
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

Generate a mixin feed based on a directory #279

Merged
merged 5 commits into from
Apr 23, 2019

Conversation

carolynvs
Copy link
Member

@carolynvs carolynvs commented Apr 18, 2019

This adds two commands to porter as a build up to supporting mixin feeds. All alone they aren't really enough to add new docs or anything yet though.

Note: This doesn't update an existing atom.xml file yet. I plan on doing that in a follow-up PR.

$ porter mixins feed template --help
Create an atom feed template in the current directory

Usage:
  porter mixins feed template
$ porter mixins feed generate --help
Generate an atom feed from the mixins in a directory.

A template is required, providing values for text properties such as the author name, 
base URLs and other values that cannot be inferred from the mixin file names. You 
can make a default template by running 'porter mixins feed template'.

The file names of the mixins must follow the naming conventions required of published 
mixins:

VERSION/MIXIN-GOOS-GOARCH[FILE_EXT]

More than one mixin may be present in the directory, and the directories may be nested 
a few levels deep, as long as the file path ends with the above naming convention, porter 
will find and match it. Below is an example directory structure that porter can list to 
generate a feed:

bin/
└── v1.2.3/
    ├── mymixin-darwin-amd64
    ├── mymixin-linux-amd64
    └── mymixin-windows-amd64.exe

See https://porter.sh/mixin-distribution more details.

Usage:
  porter mixins feed generate [flags]

Examples:
porter mixin feed generate
porter mixin feed generate --dir bin --file bin/atom.xml --template porter-atom-template.xml

Flags:
  -d, --dir string        The directory to search for mixin versions to publish in the feed. Defaults to 
the current directory.
  -f, --file string       The path of the atom feed output by this command. (default "atom.xml")
  -h, --help              help for generate
  -t, --template string   The template atom file used to populate the text fields in the generated 
feed. (default "atom-template.xml")

This template shows how to use the mustache templating
and is a working template. From there the user can customize it if they
want to use more atom features, but this is all porter needs to do its
magic.
@ghost ghost assigned carolynvs Apr 18, 2019
@ghost ghost added the review label Apr 18, 2019
Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

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

Only a couple of (non-blocking) questions; LGTM!

}

if _, err := cxt.FileSystem.Stat(o.SearchDirectory); err != nil {
return errors.Wrapf(err, "invalid --directory %s", o.SearchDirectory)
Copy link
Member

Choose a reason for hiding this comment

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

Q: should we print invalid --dir or invalid -d to represent the flag strings supported? Or perhaps just invalid directory for a more general error msg?

Copy link
Member Author

Choose a reason for hiding this comment

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

oops, yes thanks it should be --dir

return errors.Wrapf(err, "error reading template file at %s", opts.TemplateFile)
}

mixinRegex := regexp.MustCompile(`(.*/)?(.+)/([a-z]+)-([a-z0-9]+)-([a-z0-9]+)(\.exe)?`)
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to restrict some of these regex fields to only the currently supported values? (For instance, OS can be only one of {linux, windows, darwin}, arch only amd64, etc.)

I can also understand wanting to keep it unrestricted with a vision towards supporting other variants...

Copy link
Member Author

Choose a reason for hiding this comment

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

I originally didn't want to be in the business of keeping up to date with our build matrix but honestly, it is not going to change often. So we just need to be aware that this is here. I'll update it and leave a note in the makefile's build matrix that this should be updated too when it's changed.

* Limit the regex for matching mixin files to the currently supported
os/arch in our build matrix
* Fix the flag in our error message to be --dir
@carolynvs carolynvs merged commit b42a87a into getporter:master Apr 23, 2019
@carolynvs carolynvs deleted the feed-generate branch April 23, 2019 16:04
@ghost ghost removed the review label Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants