-
Notifications
You must be signed in to change notification settings - Fork 9
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
cleanup semgrep ci config section #334
Conversation
test plan: make
Backwards compatibility summary:
|
type product_ignored_files = (product * glob list) list | ||
(* We omit the usual <json repr="object"> otherwise we get a | ||
* "keys must be strings" error *) | ||
<python repr="dict"> <ts repr="map"> |
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.
We should avoid tuples in favor of records if possible. This allows future extensions, optional fields, easier debugging when working on JSON data, etc.:
type product_ignored_files = {
product: product;
ignored_files: glob list;
}
type ignored_files = product_ignored_files list
or maybe something like this:
type product_ignored_files = {
~sast: glob list;
~sca: glob list;
~secrets: glob list;
}
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.
oh nevermind, it's old code that just moved.
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.
I agree ...
test plan:
make
make setup && make
to update the generated code after editing a.atd
file (TODO: have a CI check)For example, the Semgrep backend need to still be able to consume data
generated by Semgrep 1.50.0.
See https://atd.readthedocs.io/en/latest/atdgen-tutorial.html#smooth-protocol-upgrades
Note that the types related to the semgrep-core JSON output or the
semgrep-core RPC do not need to be backward compatible!