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

feat!: vervet generate command #152

Merged
merged 3 commits into from
Mar 3, 2022
Merged

Conversation

cmars
Copy link
Contributor

@cmars cmars commented Feb 28, 2022

Instead of chaining vervet's templating capabilities on to OpenAPI
version aggregation and compilation, this breaks generators out to a
separate command a la go generate. Generators are more focused on code
generation driven by trees of OpenAPI, which "own" the generated files.

Prior experimental functionality around OpenAPI bootstrapping and
scaffolds are also removed. We want optic run scripting to manage
OpenAPI, and template projects instead of project generators.

@cmars cmars force-pushed the feat/generate-cmd branch 2 times, most recently from 93570ec to a9e1c25 Compare March 2, 2022 21:24
@cmars cmars changed the base branch from main to staging-v4 March 2, 2022 21:26
@cmars cmars force-pushed the feat/generate-cmd branch 2 times, most recently from 3578b32 to 7a28af0 Compare March 2, 2022 22:45
@cmars cmars marked this pull request as ready for review March 2, 2022 22:46
@cmars cmars requested a review from a team as a code owner March 2, 2022 22:46
internal/generator/generator.go Outdated Show resolved Hide resolved
ArgsUsage: "[api [resource]]",
Action: VersionList,
}, {
Name: "new",
Copy link
Contributor

Choose a reason for hiding this comment

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

Was dropping new support intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. vervet generate is intended to replace it. vervet version new operated in the context of a "Vervet project" owning the generators and applying them unconditionally to all resource versions. vervet generate breaks this functionality up so that you can generate from templates defined outside of the project.

This makes the generation capability more open and extensible for use outside of the context of a particular project -- templates can then be bundled and applied to any project. https://github.com/snyk/rest-node-generate will be one of these, planned rest-go-generate is another.

Also want to kill the vervet version command. Lots of feedback from the last workshop that it was confusing (it doesn't tell me the version of vervet?).

Copy link
Contributor

Choose a reason for hiding this comment

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

Make sense

@cmars cmars force-pushed the feat/generate-cmd branch 3 times, most recently from 8305473 to c4a3287 Compare March 3, 2022 18:19
Instead of chaining vervet's templating capabilities on to OpenAPI
version aggregation and compilation, this breaks generators out to a
separate command a la `go generate`. Generators are more focused on code
generation driven by trees of OpenAPI, which "own" the generated files.

Prior experimental functionality around OpenAPI bootstrapping and
scaffolds are also removed. We want `optic run` scripting to manage
OpenAPI, and template projects instead of project generators.

Drive-bys:
- Remove `vervet scaffold` command. Prefer template projects.
- Rename `vervet version` to `vervet resource` -- confusing subcommand.
  Sub-sub-commands also renamed to align with drifted purpose.
- Rename `vervet compile` to `vervet build`.
Copy link
Contributor

@jcsackett jcsackett left a comment

Choose a reason for hiding this comment

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

I've got some concerns about commands and wording, but I'm not going to block on it--I'm not at all sure that I'm correct, but I do feel like we've not quite gotten to the right terminology for end users.

cmd/cmd.go Outdated Show resolved Hide resolved
cmd/resource.go Outdated Show resolved Hide resolved
cmd/cmd.go Outdated Show resolved Hide resolved
cmd/cmd.go Outdated Show resolved Hide resolved
internal/generator/resources.go Outdated Show resolved Hide resolved
Move cmd -> internal/cmd to avoid making the command internals part of
the public API surface.

Remove the old sweater-comb linter, which used to be a Docker-packaged
Spectral linter. 'sweater-comb' is now an alias for optic-ci.
@cmars cmars merged commit f40063a into snyk:staging-v4 Mar 3, 2022
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