Skip to content

Conversation

@redsun82
Copy link
Contributor

@redsun82 redsun82 commented Apr 9, 2024

This would allow to share the patch with semmle-code without duplicating it, but as can be seen requires a lot of boilerplate, so I'm not entirely sure it's worth it.

@redsun82 redsun82 force-pushed the redsun82/kotlin-patched-registry branch 2 times, most recently from 9ffd9bc to a5324ff Compare April 9, 2024 16:07
@criemen
Copy link
Collaborator

criemen commented Apr 9, 2024

I agree, if this is the only patch we'd need to share like this, it's not worth it.

@redsun82
Copy link
Contributor Author

On the other hand, this might maybe avoid ql module sync dances down the line? Without this, we might need a semmle-code/codeql sync whenever we need to update rules_kotlin. With this, semmle-code can just have --registry=file://%workspace%/ql/misc/bazel/override_registry and not mention rules_kotlin at all. I'm a bit torn.

@redsun82
Copy link
Contributor Author

I'm leaning more and more towards this solution because of the ease it gives with respect to semmle-code/codeql syncing (or rather lack thereof). To this end I've added a script that makes the whole process of managing this local registry much easier.

What do you think @criemen?

@redsun82 redsun82 force-pushed the redsun82/kotlin-patched-registry branch 2 times, most recently from 606bb21 to 5dcb200 Compare April 10, 2024 08:26
@redsun82 redsun82 force-pushed the redsun82/kotlin-patched-registry branch from 5dcb200 to 2f7538f Compare April 10, 2024 12:57
@redsun82 redsun82 force-pushed the redsun82/kotlin-patched-registry branch from 2f7538f to 35a2ed8 Compare April 10, 2024 13:29
@redsun82 redsun82 marked this pull request as ready for review April 10, 2024 13:30
@redsun82 redsun82 requested review from a team as code owners April 10, 2024 13:30
@redsun82 redsun82 merged commit 1a7f25a into redsun82/kotlin Apr 10, 2024
@redsun82 redsun82 deleted the redsun82/kotlin-patched-registry branch April 10, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants