Skip to content

Packaging: Refactor cpp libraries #6488

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

Merged
merged 5 commits into from
Aug 17, 2021
Merged

Packaging: Refactor cpp libraries #6488

merged 5 commits into from
Aug 17, 2021

Conversation

aeisenberg
Copy link
Contributor

This PR separates the core cpp packs into codeql/cpp-queries and
codeql/cpp-all.

There are very few lines of code changed. Almost all changes are moving
files around.

Uses new style naming scheme.
@rdmarsh2
Copy link
Contributor

It looks like a handful of files under csharp/ql/lib/semmle/code/csharp/frameworks/system/data/ were accidentally committed

@aeisenberg
Copy link
Contributor Author

Thanks. You're right. Let me remove them.

This PR separates the core cpp packs into `codeql/cpp-queries` and
`codeql/cpp-all`.

There are very few lines of code changed. Almost all changes are moving
files around.
@aeisenberg aeisenberg force-pushed the aeisenberg/pack/cpp branch from 520af02 to 3b05a60 Compare August 17, 2021 18:23
We can't have recursive references to query packs.
@aeisenberg aeisenberg force-pushed the aeisenberg/pack/cpp branch from 3b05a60 to d13ce88 Compare August 17, 2021 20:04
@dbartol dbartol added the no-change-note-required This PR does not need a change note label Aug 17, 2021
Also, fix up some library path dependencies.
dbartol
dbartol previously approved these changes Aug 17, 2021
Copy link

@dbartol dbartol left a comment

Choose a reason for hiding this comment

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

LGTM.

Once we have all languages refactored, we should probably make a sweep through the whole repo to ensure that all uses of the old pack names (codeql-cpp, etc.) are gone, as well as all uses of libraryPathDependencies.

@dbartol dbartol added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Aug 17, 2021
@dbartol dbartol self-assigned this Aug 17, 2021
@aeisenberg
Copy link
Contributor Author

Yes, I was trying to make the smallest change possible at first. And we can complete the changes later.

Copy link

@dbartol dbartol left a comment

Choose a reason for hiding this comment

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

Still LGTM

@adityasharad adityasharad removed the request for review from a team August 17, 2021 23:30
@adityasharad adityasharad removed request for a team August 17, 2021 23:30
@adityasharad
Copy link
Collaborator

All checks have passed on this PR except Checks (known bug when using non-main branches) and QLDoc Checks (expected to fail since we are moving libraries around). The tests have all passed on the corresponding internal PR. Merging.

@adityasharad adityasharad merged commit 21d03cd into main Aug 17, 2021
@adityasharad adityasharad deleted the aeisenberg/pack/cpp branch August 17, 2021 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# C++ depends on internal PR This PR should only be merged in sync with an internal Semmle PR documentation Java JS no-change-note-required This PR does not need a change note Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants