Skip to content

Rust: Compute canonical paths in QL #19134

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 2 commits into from
May 22, 2025
Merged

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Mar 27, 2025

Initial implementation of canonical path computation in QL. The computed canonical paths are not yet used for anything, but they will eventually be used for mapping Models-as-Data rows to the functions that they model.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Mar 27, 2025
@hvitved hvitved force-pushed the rust/canonical-path branch 5 times, most recently from 612f797 to bd478dd Compare April 4, 2025 14:53
@hvitved hvitved force-pushed the rust/canonical-path branch 4 times, most recently from 774fa8d to bea7873 Compare April 7, 2025 18:54
@hvitved hvitved force-pushed the rust/canonical-path branch from bea7873 to c17f50a Compare May 19, 2025 08:00
filepath.matches("%/test_logging.rs") and
startline = 163
filepath.matches("%/term.rs") and
startline = [71]

Check warning

Code scanning / CodeQL

Singleton set literal Warning

Singleton set literal can be replaced by its member.
@hvitved hvitved force-pushed the rust/canonical-path branch from c17f50a to 6c007b5 Compare May 20, 2025 11:08
@hvitved hvitved marked this pull request as ready for review May 20, 2025 11:22
@hvitved hvitved requested a review from a team as a code owner May 20, 2025 11:22
@hvitved hvitved requested a review from redsun82 May 20, 2025 11:22
@hvitved hvitved force-pushed the rust/canonical-path branch from 6c007b5 to 93c8507 Compare May 21, 2025 07:22
Copy link
Contributor

@redsun82 redsun82 left a comment

Choose a reason for hiding this comment

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

I cannot comment too much on some of the QL involved (like pragmas and language annotations), but on that I will trust you 🙂 feel free to pull someone else in on that if you want another set of 👀

For the rest the general logic behind the code and the test results look good to me.

I think you might be covering more than the test is testing, so maybe consider adding more addressable things in there.

@hvitved hvitved merged commit cb59795 into github:main May 22, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants