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

making a concept abstract or changing its inheritance should be considered a breaking change #937

Open
DS-AdamMilazzo opened this issue Nov 11, 2024 · 0 comments

Comments

@DS-AdamMilazzo
Copy link

DS-AdamMilazzo commented Nov 11, 2024

Bug Report 🐛

  1. When comparing two models, Concerto doesn't consider changing a concept from concrete to abstract to be a breaking change, but it is because existing data using the concrete type will no longer validate.
  2. It also doesn't detect changes in inheritance, even when that changes the fields in a concept.

Expected Behavior

When comparing two models where the new model changes a concept from concrete to abstract, the change should be detected as a major (breaking) change. When changing the other way, from abstract to concrete, the change should be detected as a minor or patch (non-breaking) change.

When comparing two models where the new model changes a concept's inheritance, the change should be detected as a major (breaking) change. This could potentially be relaxed to a minor change if the new base type is derived (directly or indirectly) from the old base type and the newly intervening base types only add fields that would be allowed to be added directly to the concept at issue (i.e. optional fields whose names don't collide with existing fields).

Current Behavior

Currently, changing a concept between abstract and concrete is not detected as a model change, and neither is changing a concept's inheritance.

Possible Solution

Detect a change to a concept's abstractness in the model comparer and return one of two new result codes. Detect a change in base class and return a new result code (or two new result codes if you want to get fancy).

Steps to Reproduce

First, create two model files. A.cto:

namespace [email protected]
concept B { o String }
concept C extends B { }

and B.cto:

namespace [email protected]
concept B { o String }
abstract concept C { }

Then, run concerto compare --old A.cto --new B.cto
No differences are reported.

Context (Environment)

It just seems like a bug that needs fixing.

Desktop

  • OS: Windows 11
  • Browser: Chrome
  • Version: Concerto 3.16.4; Chrome 130.0.6723.117

Detailed Description

Possible Implementation

In classDeclarationTypeChanged, check if the two concepts are equally abstract and if not return new comparison result type-made-concrete (minor or patch) or type-made-abstract (major).

Also check if the base type has changed and if so return a new comparison result supertype-changed (major) (or concept-extends-changed to parallel the existing scalar-extends-changed result). If you want to get fancy, you could allow some base type changes that are not breaking, as described above in the "Expected Behavior" section.

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

No branches or pull requests

1 participant