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.

Sign up for GitHub

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

Jump to bottom

ros_comm: python "message_filters.Cache.getLastestTime" should probably be "message_filters.Cache.getLatestTime" #1449

Closed
madebyollin opened this issue Jul 6, 2018 · 3 comments
Closed

ros_comm: python "message_filters.Cache.getLastestTime" should probably be "message_filters.Cache.getLatestTime" #1449

madebyollin opened this issue Jul 6, 2018 · 3 comments

Comments

@madebyollin
Copy link
Contributor

In the C++ message_filters.Cache API there is a method called getLatestTime(). (source code)

But the python equivalent of this method is called getLastestTime(). (source code)

This seems like a typo and is potentially confusing to people moving between the two APIs. Assuming the only things that need to be changed are the function definition and the associated unit test, I can try making a PR, but it's quite a minor change so I'm not sure if that's warranted.
@dirk-thomas
Copy link
Member

A PR against the latest development branch (currently melodic-devel) would certainly be appreciated. To not break backward compatibility it would be good to keep the wrongly named function, add a note to its docblock and call the new correctly spelled function in it.

madebyollin added a commit to madebyollin/ros_comm that referenced this issue Jul 6, 2018
@madebyollin
Rename getLastestTime to getLatestTime
9429935 
Per directions [here](ros#1449 (comment)).
madebyollin added a commit to madebyollin/ros_comm that referenced this issue Jul 6, 2018
@madebyollin
Test getLatestTime instead of getLastestTime
0fa8d53 
per ros#1449 (comment)
@madebyollin madebyollin mentioned this issue Jul 6, 2018
Merged
@madebyollin
Copy link
Contributor Author

Added a PR here, although it ran into the subPub issue during CI testing 😢

dirk-thomas pushed a commit that referenced this issue Aug 3, 2018
@madebyollin @dirk-thomas
Rename python message_filters.Cache.getLastestTime to getLatestTime (#…
8341716 
…1450)

* Rename getLastestTime to getLatestTime

Per directions [here](#1449 (comment)).

* Test getLatestTime instead of getLastestTime

per #1449 (comment)
dirk-thomas pushed a commit that referenced this issue Aug 9, 2018
@madebyollin @dirk-thomas
Rename python message_filters.Cache.getLastestTime to getLatestTime (#…
f92e9e3 
…1450)

* Rename getLastestTime to getLatestTime

Per directions [here](#1449 (comment)).

* Test getLatestTime instead of getLastestTime

per #1449 (comment)
dirk-thomas pushed a commit that referenced this issue Aug 20, 2018
@madebyollin @dirk-thomas
Rename python message_filters.Cache.getLastestTime to getLatestTime (#…
b1d1dd4 
…1450)

* Rename getLastestTime to getLatestTime

Per directions [here](#1449 (comment)).

* Test getLatestTime instead of getLastestTime

per #1449 (comment)
@cwecht
Copy link
Contributor

cwecht commented May 2, 2019

This has been fixed in #1450, so this issue can be closed, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dirk-thomas @madebyollin @cwecht