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

[liamnichols]

Currently, we have a very vast amount of configuration options that must be included in the create-api.yaml file, but we also have some other options that also alter how the tool generates the output code that must be passed via CLI args. This includes the following:

• -s, --split - Split output into separate files
• --package <package> - Generates a complete package with a given name
• --module <module> - Use the following name as a module name
• --vendor <vendor> - Enabled vendor-specific logic (supported values: "github")
• --generate <generate> - Specifies what to generate (default: paths, entities)
• --filename-template <filename-template> - Example: "%0.generated.swift" will produce files with the following names: "Paths.generated.swift". (default: %0.swift)
• --entityname-template <entityname-template> - Example: "%0Generated" will produce entities with the following names: "EntityGenerated". (default: %0)

This means that there are two sources to dig through when altering configuration, which has the potential to cause confusion. It also makes it hard to invoke create-api from multiple different places without either copying the desired arguments or wrapping the invocation in a script.

Firstly, I'd like to understand if there is a good reason that these options should be separate to the options in the configuration file?

If we agree that isn't the case, I'd like to start making them available in the configuration instead.

Unless there is a good reason to keep the arguments as overrides to configuration options, my suggestion would be to deprecate them now and remove them in a future release. Printing a warning when used until they are removed.

[liamnichols]

I guess that one question could be, how many different schemas are we generating per configuration?

It might make sense to invoke the command against multiple schemas, changing only the Package name but reusing the same configuration?

But if this is the case, I might be more inclined to look for a more generic way to accept config file overrides instead?

If users were doing this, we'd find out during the deprecation window and would be able to decide accordingly then

[liamnichols]

I'm moving this up to 0.1.0 as I think it could be an important breaking change to get right. As it stands, I want to move the following:

Argument	Config
--split/-s	isMergingSources
--package	mode & target
--module	mode & target
--generate	entities.isEnabled & paths.isEnabled
--filename-template	rename.files
--entityname-template	rename.entities
--vendor	vendor

For example, the following invocation:

$ create-api generate schema.json --config .create-api.yml --package "OctoKit" --generate entities --filename-template "%0.generated.swift" --entityname-template "%0DTO" --vendor github 

Would become the following:

$ create-api generate schema.json --config .create-api.yml

.create-api.yml

mode: package
target: OctoKit
vendor: github
isMergingSources: true
paths:
  isEnabled: false
rename:
  files: "%0.generated.swift"
  entities: "%0DTO"
  properties:
    foo: bar

I'm still undecided on the file/entity templates... They feel like they should be part of renaming, and and obvious thing to do would be to support either templating or individual renames across all different types of renames. But in the event that there was somebody who relied both on --entityname-template as well as rename.entities..., they'd not be able to do both anymore. It seems pretty unlikely though?

[LePips]

Supportive of everything changed so far. Per #60 with entities and paths having separate renaming options, I simply propose:

entities:
  fileTemplate: "%0Model.swift"
paths:
  fileTemplate: "%0API.swift"

I think we can even move rename.properties, rename.parameters, rename.entities, and rename.operations into their respective sections:

entities:
  fileTemplate: "%0Model.swift"
  renameEntities: ...
  renameProperties: ...
paths:
  fileTemplate: "%0API.swift"
  renameOperations: ...
  renameParameters: ...

This still leaves rename.enumCases and rename.collectionElements but I can't think of a good moving scheme for them right now.

Edit:
Change the last yaml to:

entities:
  fileTemplate: "%0Model.swift"
  rename:
  - User: Person
  - Account.description: Overview
paths:
  fileTemplate: "%0API.swift"
  rename: ...

[liamnichols]

I think (but didn't confirm) that property renaming applies to query parameters too, so maybe three still need to be generic 😄

I am +1 for entities.filenameTemplate and paths.filenameTemplate though. These can replace --filename-template

I guess I still need to figure out what to do with --entityname-template though 😄

[LePips]

We can just simply remove it as it will be entities.renameEntities and that new name take place of %0.

Edit: My bad, the template and renaming entities aren't really the same. Do we need to have an entity template though? I assume that objects are properly contextually named ("__item", "__object", "__model") so having a template doesn't serve much purpose for the actual entity types*. We could look at re-adding it if somebody really requests it.

*other than avoiding file conflicts with paths but this is no longer necessary with #108

[LePips]

As an additional reason for --entityname-template to be removed: it does not work for when a Path references the object*. The context of the entity template renaming is held within each individual generator. Meaning when Paths are generated it does not know of any entity templates.

*Using --entityname-template "%0Model.swift" will rename Pet to PetModel. However, a call that will return a Pet schema object will still expect the original entity type Pet.

[liamnichols]

Is that the case? At Cookpad we have been using --entityname-template "%0DTO" without an issue and I think it needs to remain unless there is an alternative way to prefix/suffix all generated entities?

[LePips]

Apologies, ignore that. I have edited my local petstore schema for testing and got mixed up with the provided Pets declaration which I have removed. It does properly rename with the template.