-
Notifications
You must be signed in to change notification settings - Fork 30
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
Fix/3193/custom config todo due #3196
Conversation
update to the babel suite
add proper error handling add (bad) if condition for tests in customConfigHelper.ts
add proper error handling add (bad) if condition for tests in customConfigHelper.ts
…into fix/3193/customConfig-todo-due
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some considerations I think that we should just remove the transform and a check if the config is conform with the supported config strucutre is also not needed as long as the system does not crash. If it would crash we should handle the error gracefully.
What is happening when loading/importing a config with old camera properties? I guess, they are just ignored and a default camera position is set instead. in that case, I would tolerate this because such deprecated configs must have been created before releasing the feature as stable (so we do not necessarily have to support the transformation of "unstable" configs).
The reason for rollbacking the before discussed solution is that in the future we should handle backwards incompatible config structure changes by the config structure api version and add a appropriate validation method for applying/importing configs.
It looks like this has been forgotten in #2880, to better signal incompatibility it is now changed
also revert changes to applyCustomConfigButton.component.ts and uploadCustomConfigButton.component.ts
The changes have been made, and everything seems to run bug free on my machine. The camera position is now simply ignored. I have also bumped the api version of the config files to better signalise the change. |
Let's not bump the version because the deprecation of theese properties occurred when the Custom Views feature was not released stable. It is not ideal that we are already at version 1.0.0. Before releasing the feature as stable the structure should have been at 0.x.x. Any change that will break the support of a config from now on is going to bump the mature version, because the feature is stable. |
[CodeCharta Analysis] Kudos, SonarCloud Quality Gate passed! |
[CodeCharta Visualization] Kudos, SonarCloud Quality Gate passed! |
The removal of the camera attribute migration for custom view files is due.
Issue: #3193
Description
In #2880 the way camera position and angle were saved changed in an incompatible way. To give everyone some time to transition, a mapping method was introduced to parse the old camera attributes into the new format.
Now, the time has come for its removal, the camera position from old files will now simply be ignored.
This affects all cc.config files generated with CodeCharta 1.101.1 and earlier.
This PR also updates the babel suite because tests were failing locally on my machine.