-
Notifications
You must be signed in to change notification settings - Fork 14
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
Revit_Engine: Improve error message when diffing with duplicate elementIds #1247
Revit_Engine: Improve error message when diffing with duplicate elementIds #1247
Conversation
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.
Some optional changes requested - happy to chat.
Co-authored-by: Alessio Lombardi <[email protected]>
Thanks @alelom ! Have commited your suggested change with a minor fix and fixed a typo in the comments :) |
Co-authored-by: Alessio Lombardi <[email protected]>
I've added a test for this PR in DiffingTests_Prototypes. @IsakNaslundBh @pawelbaran I ran all the tests to verify that nothing is broken, all happy. If you can, please try running them too -- should be our procedure to test for regressions with diffing, in general. |
@alelom I have run through all tests, with all passing. If you are happy, would you mind approving so that we can get this merged? :) |
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.
Approved based on changes, discussion and tests.
@BHoMBot check required |
@IsakNaslundBh to confirm, the following actions are now queued:
|
@pawelbaran to confirm, the following actions are now queued:
There are 21 requests in the queue ahead of you. |
Issues addressed by this PR
Closes #1246
Improves the error message given when duplicate ids is detected. Gives further guidance to use physical elements rather than structural if structural elements are detected.
Changing the Diffing method called to one one step deeper as ids are already extracted.
Test files
(from @alelom) added a test for this PR in DiffingTests_Prototypes. Run all tests in that solution to verify this PR.
Changelog
Additional comments