-
Notifications
You must be signed in to change notification settings - Fork 568
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
[Cairo 1.0] Add transform Tree-Sitter -> Generic AST #7757
Conversation
|
||
(*****************************************************************************) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a toplevel comment explaining the goal of the module and how it came to be.
For example did you start by copy-pasting code from Parse_rust_tree_sitter.ml?
Or you wrote it from scratch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I copy-pasted everything that was below entry point and I've written everything else from scratch.
When you says "how it came to be" you mean the reason why we wanted to have Cairo 1.0 supported by Semgrep ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I meant how was the code written. For example if lots of it was copy-pasted from Parse_rust_tree_sitter.ml,
that can be useful to know for the reader, because maybe some further update of Rust will lead to further
update of Cairo in which case we could just copy-paste again code from Parse_rust_tree_sitter.ml to Parse_cairo_tree_sitter.ml to save time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it fine like that ?
you also need to install pre-commit and the right version of ocamlformat. |
you can try 'make dev-setup' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need pre-commit and ocamlformat compliant code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perfect
PR checklist:
If you're unsure about any of this, please see: