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

Change rename.properties to use schema names #112

Closed
LePips opened this issue Aug 5, 2022 · 1 comment · Fixed by #133
Closed

Change rename.properties to use schema names #112

LePips opened this issue Aug 5, 2022 · 1 comment · Fixed by #133

Comments

@LePips
Copy link
Contributor

LePips commented Aug 5, 2022

I observe in this comment that rename.properties requires the format of the post-processed entity name that will be attached to the final entity.

Example

The following config:

Pet:
  properties:
    example_name:
      type: string
    enabled:
      type: boolean
    source_url:
      type: String

will result in the entity:

struct Pet {
    var exampleName: String
    var isEnabled: String // due to useSwiftyPropertyNames
    var sourceURL: String // due to isReplacingCommonAcronyms
}

When a developer uses rename.properties, they are required to use the final processed entity name for the key and the value is a literal translation [1], no processing is done on the renamed property. Additionally, the developer has to know what acronyms are used and the states of the pluralizeProperties, and useSwiftyPropertyNames. This can cause a lot of problems during renaming and I omit examples of mixture of these properties because it's pretty straightforward.

rename:
  properties:
    Pet.isEnabled: somethingElse
    Pet.sourceURL: another_url # will literally become `another_url` on the entity

Change

To be consistent with include/exclude and to be more clear with how the config should be created, we should instead change rename.properties to use the schema object names for keys and values.

rename:
  properties:
    Pet.enabled: something_else
    Pet.source_url: another_url

would result in the entity:

struct Pet {
    var exampleName: String
    var somethingElse: String
    var anotherURL: String
}

  • [1] I don't think that literal translations should be an intended use of rename.properties. If a developer really really wanted to break their own convention we could implement some kind of literal renaming option that ignores the naming processing steps. I think a comment would be very clear and declarative, but I don't know how the YAML would parse that at the moment:
rename:
  properties:
    Pet.enabled: something_else
    Pet.source_url: another_url # literal
@LePips LePips changed the title Change rename.properties to use Schema names Change rename.properties to use schema names Aug 5, 2022
@liamnichols
Copy link
Member

Yeah this makes sense to me. I think everything in the config that relates to a entity/property/operation etc should always match the values defined in the schema before any transformations are applied.

Because most of the config options were added iteratively after the initial implementation, things were just built on top of each other which is why there are some inconsistencies. I think there is probably some refactoring to be done internally to clean this up a lot 👍

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 a pull request may close this issue.

2 participants