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

[DROOLS-7577] drools-lsp : Add comments and explanations for drools-p… #43

Merged
merged 2 commits into from
Nov 17, 2023

Conversation

tkobayas
Copy link
Collaborator

@tkobayas tkobayas commented Nov 7, 2023

…arser

@tkobayas
Copy link
Collaborator Author

tkobayas commented Nov 7, 2023

@gitgabrio @mariofusco @yurloc Feel free to let me know if you want more explanation about the parser implementation

Comment on lines 161 to 164
public FunctionDescr visitFunctiondef(DRLParser.FunctiondefContext ctx) {
// function String functionA(String s, Integer i) {
// foo();
// }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Example syntax as a comment. Is this too much? But maybe helpful for new contributors.

Copy link
Contributor

Choose a reason for hiding this comment

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

hi @tkobayas
TBH I'm not so sure. On one side, I'm afraid that in the long term there could be more comments then actual code and, paradoxically, this could decrease readability. On the other side, it is slightly confusing because it is not clear (at least to me) if the function must have exactly that signature, or if it can be any possible function.
Maybe for such low level details it would be better to create a sort of readme/manual/developer guide: wdyt ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Syntax details are written in documentation. The comments are just examples to help developers, but I agreed that it could decrease readability. Removed the syntax example comments. Thanks, @gitgabrio

@mariofusco mariofusco merged commit 4cf1bf5 into kiegroup:main Nov 17, 2023
2 of 3 checks passed
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