-
Notifications
You must be signed in to change notification settings - Fork 20
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
Refactoring and pydoc comments #38
Conversation
@Guelakais could you undo all the code changes and only leave the comments? Your code changes are making the lint checker fail (see https://github.com/colcon/colcon-cargo/actions/runs/9723715491/job/27488484940?pr=38) |
yep |
I am personally not in favor of this kind of changes, the docstrings are very verbose and in most cases don't really add any information. In my opinion a docstring should document the purpose of the function, maybe a brief summary and any additional information that is not obvious from looking at the code, this is not really the case for most (any?) of the changes here. For example:
I would advise against adding a docstring to every single function just because public functions should have documentation. Instead I would only add docstrings to more complex functions to document what is not obvious by looking at the code. |
In exactly the same way, you could simply remove the methods that do nothing. In case of doubt, it is not obvious exactly what a method does, as neither the input nor the output type are explicitly described. You can also save yourself a lot of documentation if you simply write the corresponding data type after each input parameter and the output type at the end of the method with I've just been looking for a good example of what I mean. The following method doesn't really need any additional comments, as it's perfectly clear what's going on. It is clear at all times what goes into the method and what comes out of the method. It's Python, by the way. def create_point(x_axe:float,y_axe:float,z_axe:float) -> Point:
point=Point()
point.x=x_axe
point.y=y_axe
point.z=z_axe
return point |
@Guelakais the problem is that many of the comments you've added don't describe the intent of the method, but just feel like they are a line by line read of the code, for example: How does this comment improve readability? It does is just describe each line of code, but nothing else. |
made some refactorings to make the code a little bit cleaner. Mosly I've added comments.