Skip to content

Commenting step 1 - #39

Open
ghost wants to merge 7 commits into
Devfrom
Commenting-Step-1
Open

Commenting step 1#39
ghost wants to merge 7 commits into
Devfrom
Commenting-Step-1

Conversation

@ghost

@ghost ghost commented Apr 8, 2024

Copy link
Copy Markdown

Pull Request for Commenting Branch

@ghost
ghost requested a review from dlezcan1 April 8, 2024 22:29
@ghost ghost self-assigned this Apr 8, 2024

@dlezcan1 dlezcan1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I do not directly understand what this Explanations static class is supposed to do and what the "Explanation" functions are supposed to do.

Comment on lines +157 to +161

/** <b> DETAILED EXPLANATION </b> */
public static int Explanation() {
return 1;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I Created a function called explanation in each class. in the explanation file people will get a clear explanation of how things work based off of the java doc of that explanation

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is there a function rather a direct comment block with the class and comment blocks for each of the functions? Why use this arbitrary Explanation function?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I didnt want to make the explanation for the class to long

@dlezcan1 dlezcan1 Apr 8, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you should focus more on comment blocks, rather than this Explanation class system. I would revert these commits and go with comment blocking

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Also We need to have explanations for things like init Sendable and command schedule and auto etc

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Those can be done for the initSendable functions, individually

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

What about command Scheduler etc. I want to make it so that each student who wants to code after me knows exactly where to look for stuff. I cant java doc the init sendable stuff really and command scheduler stuff

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Take this to the main comment block. I'll respond there.

Comment thread src/main/java/frc/robot/Other/Explanations.java
*/
public class Explanations {

public abstract class SubsystemExplanations {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is the public abstract class?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Dont wont to make an object of it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why make an object out of it at all? Why is this a class?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

want*

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right, why is this a class at all?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I wanted to make it clear for younger students where all the explanataions would be

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would argue that the class implementation is more confusing itself. I think this would be more smoothly implemented with your structure you have in Hanger.java minus the Explanation function.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

When people want explanation for things I would like for everything to be in the same place

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

If you look in Hanger Module The explanation is like a good 200 words that I dont want to be shown when hovering over HangerModule.java

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Break the comments into their individual functions then.

* Resets the Position of the Odometer, given our Current position.
*
* @param Pose2d (pose2d) - The current position of the robot on the field. This is a {@link
* @param pose2d (pose2d) - The current position of the robot on the field. This is a {@link

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Typical format is param name (Parameter's Class). change pose2d (pose2d) to pose2d (Pose2d)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I intended to do that but misclicked apparently

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Got it. Will resolve comment when addressed.

@dlezcan1 dlezcan1 linked an issue Apr 8, 2024 that may be closed by this pull request
@dlezcan1

dlezcan1 commented Apr 8, 2024

Copy link
Copy Markdown
Member

@jforchheimer25 Thread here: I believe the structure of an abstract static class is more confusing than just using standard comment blocks for all functions and classes

@dlezcan1

dlezcan1 commented Apr 8, 2024

Copy link
Copy Markdown
Member

When people want explanation for things I would like for everything to be in the same place

@jforchheimer25
Then you can do that 1 of two ways. 1: A separate .java file with comment block descriptions, or a README.md file which connects all of these together. A class with arbitrary functions which return arbitrary integers is confusing and clunky. It's sleeker to stick to comment blocking

@dlezcan1

dlezcan1 commented Apr 8, 2024

Copy link
Copy Markdown
Member

What about command Scheduler etc. I want to make it so that each student who wants to code after me knows exactly where to look for stuff. I cant java doc the init sendable stuff really and command scheduler stuff

This can be commented where it is implemented. We could have a separate function on each and link them to the web pages with our simpler explanation

@ghost

ghost commented Apr 8, 2024

Copy link
Copy Markdown
Author

This can be commented where it is implemented. We could have a separate function on each and link them to the web pages with our simpler explanation

could you show an example?

@dlezcan1

dlezcan1 commented Apr 8, 2024

Copy link
Copy Markdown
Member

Here is an example for Python of good structured commenting:

https://github.com/PhotonVision/photonvision/blob/master/photon-lib/py/photonlibpy/photonPoseEstimator.py

@ghost

ghost commented Apr 8, 2024

Copy link
Copy Markdown
Author

Here is an example for Python of good structured commenting:

https://github.com/PhotonVision/photonvision/blob/master/photon-lib/py/photonlibpy/photonPoseEstimator.py

Ill Remove the class later then. Im more focused on getting the explanations in first.

@dlezcan1

dlezcan1 commented Apr 8, 2024

Copy link
Copy Markdown
Member

Sure, but keep new additions to be class-based and function-based. No need for the Explanation functions.

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.

Documentation of Code

1 participant