search

stack-of-tasks / pinocchio

A fast and flexible implementation of Rigid Body Dynamics algorithms and their analytical derivatives
http://stack-of-tasks.github.io/pinocchio/
BSD 2-Clause "Simplified" License
1.81k stars 381 forks source link

Minor syntax error #1153

Closed duburcqa closed 4 years ago

duburcqa commented 4 years ago

I just wanted you to know that there is a syntax error in this header, even though it is compiling without error with the right compilation flags.

https://github.com/stack-of-tasks/pinocchio/blob/cd452efbeb3f04dc980def5fe0fc4a14edc197b7/src/parsers/urdf/types.hpp#L68

jmirabel commented 4 years ago

Thanks. Can you provide the compilation error please ?

jcarpent commented 4 years ago

@duburcqa Thanks for the pointer to the syntax error.

jmirabel commented 4 years ago

Arf, that was too obvious... maybe we should use something like https://www.codacy.com/.

jmirabel commented 4 years ago

@jcarpent would you agree with that ? I can try to set it up.

jcarpent commented 4 years ago

@jmirabel Yes please go ahead. It it help us to prevent such stupid things, I'm not against at all. Thanks a lot

jcarpent commented 4 years ago

Solved by #1154

jmirabel commented 4 years ago

I made a request but I don't know who receives it (send to @stack-of-task ...).

florent-lamiraux commented 4 years ago

I did.

florent-lamiraux commented 4 years ago

I have accepted. Is it all right ?

jmirabel commented 4 years ago

They tell me

Seems like you are not an organization owner. You need to request an owner to join Codacy and add the organization.

So @florent-lamiraux , you still have an action to do.

jmirabel commented 4 years ago

Thank you. It worked.

jmirabel commented 4 years ago

Here is the result: https://app.codacy.com/gh/stack-of-tasks/pinocchio/dashboard

It will be run on every PR (and commit ?).

cmastalli commented 4 years ago

This tool is cool! @jcarpent, @nim65s could we allow it on loco-3d? I would like to use it in Crocoddyl.

nim65s commented 4 years ago

Arf, that was too obvious... maybe we should use something like https://www.codacy.com/.

Codacy automatically run a bunch of linters on your code. It is not a linter by itself, is it ? So which static analysis tool do you think would have prevented such a typo ?

jmirabel commented 4 years ago

I think it isn't a linter by itself.

I ran cppcheck and it does not report it. I was probably overoptimistic then.

cmastalli commented 4 years ago

I though it was more than a linter, right?

It is just a linter then we don't need it. I would follow the linter project developed by @nim65s, it is quite useful :)

jmirabel commented 4 years ago

At least, it didn't find this issue: https://app.codacy.com/gh/stack-of-tasks/pinocchio/file/43958393041/issues/source?bid=17605373&fileBranchId=17605373

nim65s commented 4 years ago

As far as I can see, it is not a linter, it's a tool that run external linters on each commit / PR and show the results in the commit / PR, and in a dashboard.

jmirabel commented 4 years ago

@nim65s would you know a tool that would be able to catch this kind of typo ? Basically, a compiler which wouldn't do anything else than parsing the code (though I don't know how faster this would be).

nim65s commented 4 years ago

I don't know one which would have automatically tested this with and without PINOCCHIO_URDFDOM_USE_STD_SHARED_PTR

nim65s commented 4 years ago

But I plan on adding clang-tidy & cppcheck everywhere :)

jmirabel commented 4 years ago

Well, if there exists a tool as fast as cppcheck but that would take the same option as a compiler (the preprocessor variables in this case), one can easily image running this tool with all the possible flags combinations.

nim65s commented 4 years ago

I will try to do that :)