-
Notifications
You must be signed in to change notification settings - Fork 0
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
Importing ROS2 messages into Capella model #3
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.
Quite some things to do. If you also want to do some more refactoring to standardize the methods on the converters, we can hop into a pair-programming session.
18f6896
to
d866404
Compare
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.
The problem with filenames coming from class names including a /
in the export is also apparent in the import. This needs to be handled both ways.
When this is done: LGTM.
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.
Look for the type annotations also in the test modules.
After my requested changes: LGTM.
0ec3d8e
to
f6e3152
Compare
The pull request does not conform to the conventional commit specification. Please ensure that your commit messages follow the spec: https://www.conventionalcommits.org/. This is the commit validation log:
Here are some examples of valid commit messages:
|
And also, please fix the commit messages. The bot is slowly approaching the point where funny turns into annoying. :) |
fb25fb3
to
120d1c8
Compare
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.
Really good! I like how far we are here. After my tiny requested changes: LGTM.
Co-authored-by: Martin Lehmann <[email protected]>
Co-authored-by: Martin Lehmann <[email protected]>
No description provided.