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

Allow lowercase transformers key #665

Merged
merged 18 commits into from
May 4, 2022
Merged

Allow lowercase transformers key #665

merged 18 commits into from
May 4, 2022

Conversation

olblak
Copy link
Member

@olblak olblak commented Apr 25, 2022

Fix #655
Fix #653

  • Convert transformer to a struct which allows to generate jsonschema fields
  • Deprecate non lowercase transformer fields such as semverInc becomes semverinc

Test

To test this pull request, you can run the following commands:

cp pkg/core/transformer
go test
 go build -o bin/updatecli && ./bin/updatecli jsonschema --directory schema

Ensure that jsonschema contains transformers field

Additional Information

Tradeoff

A side effect of converting transformer to a stuct means that the two following examples are accepted and means different things

*Order is guaranteed because of the list

# v1.0.0-alpha
transformers:
    - trimsuffix: alpha
    - replace:
          from: alpha
          to: beta

Order is not guaranteed because of the map

# v1.0.0-alpha
transformers:
    - trimsuffix: alpha
      replace:
          from: alpha
          to: beta

Potential improvement

Deprecate non lowercase transformer field

Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
@olblak olblak added enhancement New feature or request jsonschema Anything related to updatecli jsonschema labels Apr 25, 2022
@olblak
Copy link
Member Author

olblak commented May 2, 2022

Considering that this pullrequest is fully backward compatible, I am going to merge it and proceed.
The three main advantages of this pulllrequest

  1. Allow to use lowercase transform key so addprefix instead of addPrefix which is more consistent with the naming convention used now
  2. Generate jsonschema for transformer which is more convenient when writing Updatecli manifest from IDE support jsonschema store

olblak and others added 10 commits May 2, 2022 20:34
Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
Signed-off-by: Olblak <me@olblak.com>
@olblak
Copy link
Member Author

olblak commented May 3, 2022

for context, I move the updatecli manifest in the e2e directory under e2e/updatecli.d so Vscode could enable jsonschema validation

@olblak
Copy link
Member Author

olblak commented May 4, 2022

After adding e2e tests for transformers where I test both the old and new syntax. I consider this ready to be merged

@olblak olblak added the transformers Related to transformers label May 4, 2022
@olblak olblak merged commit 65f6aa6 into updatecli:main May 4, 2022
@olblak olblak deleted the issues/655 branch May 4, 2022 06:56
@olblak olblak changed the title Add transformers field to jsonschema Allow lowercase transformers key May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request jsonschema Anything related to updatecli jsonschema transformers Related to transformers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Transformer key should all be lowercase Jsonschema doesn't include transformers spec
1 participant