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.

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

point implementation #19

Merged
merged 7 commits into from Aug 26, 2019

Conversation

@ThibaultLacharme
Copy link
Contributor

commented Aug 6, 2019

No description provided.

@lgtm-com

This comment has been minimized.

Copy link

commented Aug 6, 2019

This pull request introduces 1 alert when merging 904fae1 into e68843e - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@ThibaultLacharme ThibaultLacharme force-pushed the ThibaultLacharme:point_stamped branch from 904fae1 to 2b8e53f Aug 6, 2019

@lgtm-com

This comment has been minimized.

Copy link

commented Aug 6, 2019

This pull request introduces 2 alerts when merging 2b8e53f into e68843e - view on LGTM.com

new alerts:

  • 2 for Unused variable, import, function or class

@ThibaultLacharme ThibaultLacharme force-pushed the ThibaultLacharme:point_stamped branch from 2b8e53f to f04b14d Aug 6, 2019

@ThibaultLacharme ThibaultLacharme force-pushed the ThibaultLacharme:point_stamped branch from f04b14d to dd8fb4e Aug 6, 2019

@lgtm-com

This comment has been minimized.

Copy link

commented Aug 6, 2019

This pull request introduces 1 alert when merging dd8fb4e into e68843e - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class
@chaitanya-deep
Copy link
Member

left a comment

Hi @ThibaultLacharme ,
Thank you for adding the class. I requested a few changes.
Functionally I tested it and works perfect. It's mostly minor syntactic changes for consistency.

src/primitives/Sphere.js Outdated Show resolved Hide resolved
src/viz/Point.js Outdated Show resolved Hide resolved
src/utils/constants.js Outdated Show resolved Hide resolved
src/primitives/Sphere.js Outdated Show resolved Hide resolved
@chaitanya-deep chaitanya-deep referenced this pull request Aug 7, 2019

@ThibaultLacharme ThibaultLacharme force-pushed the ThibaultLacharme:point_stamped branch from dd8fb4e to 20c9912 Aug 7, 2019

@chaitanya-deep chaitanya-deep requested a review from tocttou Aug 24, 2019

@lgtm-com

This comment has been minimized.

Copy link

commented Aug 24, 2019

This pull request fixes 1 alert when merging e03e133 into a23a87a - view on LGTM.com

fixed alerts:

  • 1 for Expression has no effect
@tocttou
Copy link
Collaborator

left a comment

@ThibaultLacharme Thanks. This PR seems to have a minor merge conflict due to a variable name change in devel. Can you please update your branch for that?

@chaitanya-deep chaitanya-deep requested a review from tocttou Aug 26, 2019

@tocttou tocttou merged commit 8a803c0 into rapyuta-robotics:devel Aug 26, 2019

2 checks passed

LGTM analysis: JavaScript No new or fixed alerts
Details
Travis CI - Pull Request Build Passed
Details

@ThibaultLacharme ThibaultLacharme deleted the ThibaultLacharme:point_stamped branch Aug 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.