Skip to content

fix: Merging converters#152

Draft
ewuerger wants to merge 2 commits intomainfrom
fix-converter-config
Draft

fix: Merging converters#152
ewuerger wants to merge 2 commits intomainfrom
fix-converter-config

Conversation

@ewuerger
Copy link
Copy Markdown
Collaborator

@ewuerger ewuerger commented Mar 7, 2025

No description provided.

@ewuerger ewuerger self-assigned this Mar 7, 2025
@ewuerger ewuerger added the bug Something isn't working label Mar 7, 2025
@ewuerger ewuerger requested a review from micha91 March 10, 2025 09:10
Copy link
Copy Markdown
Collaborator

@micha91 micha91 left a comment

Choose a reason for hiding this comment

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

To my understanding, this PR merges add_attributes serializer configs when it comes to hierarchical configurations. I don't think that we should do that, because we loose this way the ability to define that an attribute in some cases might not be wanted in a specific type. If we want to be able to stack configurations, we should do it differently. We could for example allow configurations like this:

"*":
    "*":
      serializer:
        add_attributes-general:
          - capella_attr: layer
            polarion_id: layer
sa:
    PhysicalComponent:
      serializer:
        add_attributes:
          - capella_attr: nature
            polarion_id: nature

The general rule would be that the serializer can be suffixed using a -<suffix>. These serializers could on lower level be overwritten as they are currently overwritten, but you can also have multiple configurations of the same serializer in your config. This would also enable you to define an add_jinja_fields serializer on a on abstract level and another one as add_jinja_fields-custom on a specific level, if you want to add another custom field

@ewuerger ewuerger marked this pull request as draft June 3, 2025 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants