Skip to content

Feature/transform#17

Merged
emersonknapp merged 7 commits into
ros-tooling:mainfrom
wep21:feature/transform
Nov 23, 2021
Merged

Feature/transform#17
emersonknapp merged 7 commits into
ros-tooling:mainfrom
wep21:feature/transform

Conversation

@wep21
Copy link
Copy Markdown
Contributor

@wep21 wep21 commented Nov 18, 2021

wep21 added 2 commits November 19, 2021 03:22
Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>

Add transform

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
@wep21 wep21 requested a review from a team as a code owner November 18, 2021 18:28
Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
@wep21 wep21 force-pushed the feature/transform branch from 29d2c94 to 29f0568 Compare November 18, 2021 18:31
@emersonknapp
Copy link
Copy Markdown
Member

My major concern here is that the Python linters are not run on this script - probably because it doesn't end in .py. I'd like to make sure to apply standard ROS 2 linters to all code here, at the very least (an actual functional test would be nice but I won't block on that).

Could you make it so that the linters run on this new source? Probably you could strip the .py extension and do a chmod on the file as part of the CMake install step.

wep21 added 3 commits November 19, 2021 16:42
Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
@wep21 wep21 force-pushed the feature/transform branch from c23e613 to efb3945 Compare November 19, 2021 07:42
Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
@wep21
Copy link
Copy Markdown
Contributor Author

wep21 commented Nov 19, 2021

Copy link
Copy Markdown
Member

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

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

Thanks for making the change - I think this looks good to me

@emersonknapp emersonknapp merged commit 55f7f25 into ros-tooling:main Nov 23, 2021
@wep21 wep21 deleted the feature/transform branch November 23, 2021 20:20
@mateusz-lichota
Copy link
Copy Markdown
Contributor

@emersonknapp @wep21 , the fact that SCRIPTS_DESTINATION is used means that the package doesn't build correctly on galactic. Is that the desired behavior?

@wep21
Copy link
Copy Markdown
Contributor Author

wep21 commented Nov 26, 2021

@mateusz-lichota I created a PR to address the isseu you suggested. Please check it out. #18

@emersonknapp
Copy link
Copy Markdown
Member

Thanks for the work @wep21 - I have merged that PR

wep21 added a commit to wep21/topic_tools that referenced this pull request Dec 9, 2021
This reverts commit 4151102.

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
wep21 added a commit to wep21/topic_tools that referenced this pull request Oct 8, 2022
This reverts commit 4151102.

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
wep21 added a commit to wep21/topic_tools that referenced this pull request Oct 8, 2022
This reverts commit 4151102.

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
wep21 added a commit that referenced this pull request Oct 20, 2022
This reverts commit 4151102.

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
Signed-off-by: Daisuke Nishimatsu <border_goldenmarket@yahoo.co.jp>
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.

3 participants