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

Move generator related CLI options into configuration file #134

Merged
merged 6 commits into from
Aug 13, 2022

Conversation

liamnichols
Copy link
Member

@liamnichols liamnichols commented Aug 10, 2022

The --merge-sources, --package, --module, --vendor, --generate and --entityname-template options all alter the generator output but they are currently passed separately to the rest of the configuration options.

As part of the linked ticket, we want all options that alter the behaviour of the generator to be sourced from the ConfigOptions type which loads primarily from the .create-api.yml configuration file. Doing so would allow the user to store all important information in a single place without having to worry about making sure that these values are always passed when they invoke the cli by wrapping in a script etc.

In cases where the user might actually want to pass these kind of options, or others that were previously only defined in the configuration file, we've introduced the general purpose --config-option option that allows you to pass a key.path=value pair. See #126 for more info.

Refer to the table below for a mapping of the options

Before After
--merge-sources mergeSources
--package generate and module
--module generate and module
--vendor vendor
--generate generate
--entityname-template entities.nameTemplate

For more information, keep reading

mergeSources

This one is pretty straightforward, it's just a boolean flag at the top level:

mergeSources: true 

The default value is false

generate and module

These two options differ slightly to how they were previously used since they are no longer mutually exclusive. generate is now a set with three options (paths, entities, and package) and module is the name of the module being generated.

module is required and generate defaults to [entities, paths, package]. So by default a complete package is generated, and all you need is to specify the module name:

module: MyCustomAPIKitInAPackage

If you don't want the package, you'd do something like this:

module: MyAPIKitInAPackage
generate: [entities, paths, enums]

Note that isGeneratingEnums has also been incorporated into the generate option.

vendor

Pretty straightforward although this is now an enum so you can't specify anything other than github or null.

vendor: github

entities.nameTemplate

Finally, the --entityname-template option has moved into entities like so:

entities:
  nameTemplate: %0DTO

Feedback on the approach is welcome, because there may well be a better way to approach this.

name: "test-query-parameters",
arguments: [
"--package", "test-query-parameters"
]
name: "test-query-parameters"
Copy link
Member Author

@liamnichols liamnichols Aug 10, 2022

Choose a reason for hiding this comment

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

In GenerateTestCase's snapshot method, I've updated it to infer the moduleName from the name parameter. This means that in a lot of places we no longer need to pass the moduleName so you'll see that --package turns into nothing in the diff since format: package is default

@LePips
Copy link
Contributor

LePips commented Aug 11, 2022

Continued from #85.


  • The package.dependencies structure sounds good, so my PR can wait until this one is merged. I think it wouldn't hurt to have packages.name as it's a mixed bag for whether the package name should match a module name in a lot of SPM packages.
  • The generate: entities, paths also looks good
  • I don't think that the concept of enabled fits in a config: options specified in a config should happen (unless overridden or incorrect) and I don't fancy the idea of "disabling" a section if it was written out. So, I prefer the above option of generate.

@liamnichols
Copy link
Member Author

I think it wouldn't hurt to have packages.name as it's a mixed bag for whether the package name should match a module name in a lot of SPM packages.

Yeah, in most cases you typically name the package after the name of the product that it exports (which is the target name) and we do this already. But there are cases where it differs.

For example, the swift-argument-parser package has a target called ArgumentParser. If package.name is unset, we can continue with the legacy behaviour 👍

I won't tackle that in this PR though

@liamnichols
Copy link
Member Author

@LePips: I pushed up the changes, since I'm removing the enabled flags for the single generate and am not adding package.name here, I've not actually added the package option so you can add it directly in your PR.

I can add package.name afterwards 👍

@liamnichols liamnichols force-pushed the ln/move-cli-options branch 2 times, most recently from 6844995 to 0a50d14 Compare August 11, 2022 18:41
@liamnichols
Copy link
Member Author

As per #130 (comment), I've also included enums as part of the generate option set.

@liamnichols liamnichols merged commit d1cd552 into main Aug 13, 2022
@liamnichols liamnichols deleted the ln/move-cli-options branch August 13, 2022 16:08
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.

All command line arguments that alter output should be configurable in config file
2 participants