Skip to content

Origin/feature/omh to ros dev#2

Open
moperezs wants to merge 25 commits intomainfrom
origin/feature/omhToRosDev
Open

Origin/feature/omh to ros dev#2
moperezs wants to merge 25 commits intomainfrom
origin/feature/omhToRosDev

Conversation

@moperezs
Copy link
Collaborator

No description provided.

Copy link
Collaborator Author

@moperezs moperezs left a comment

Choose a reason for hiding this comment

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

Great work!! It already looks very gueet.
Some general comments:

  • Please add an empty line at the end of each file
  • Check that all functions have types in inputs and expected outputs
  • Did you format the files (e.g. with blackformatter or mypy)? Some lines seem quite long here in git.
  • I moved the omh_streamer to the root directory, because it just streams data, which i would separate from converting data.
  • From my point of view, conversions should work online and offline. The user should choose whether they implement the library in either case. But I see here you separated online and offline, is there a reason for it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would remove this .gitignore from the repo and let everyone have their custom configs if needed.

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

Successfully merging this pull request may close these issues.

2 participants