-
Notifications
You must be signed in to change notification settings - Fork 113
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
Q# Package System #1698
Q# Package System #1698
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.
Finished reviewing, changes look good, a few minor things called out. The only major thing is a possible side effect of removing visibility from the AST precluding future lints, but even that can be something we add back in later in a limited fashion if it makes sense to do so.
Change in memory usage detected by benchmark. Memory Report for e1e316f
|
Benchmark for e1e316fClick to view benchmark
|
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 finished reviewing and the changes look good. I left a few comments with mostly non-blocking things, but it would be good to address them before merging.
Change in memory usage detected by benchmark. Memory Report for c83314c
|
Change in memory usage detected by benchmark. Memory Report for 3aff1d9
|
Benchmark for 3aff1d9Click to view benchmark
|
Change in memory usage detected by benchmark. Memory Report for 1c15995
|
Change in memory usage detected by benchmark. Memory Report for e56ee3a
|
Benchmark for 1c15995Click to view benchmark
|
Benchmark for e56ee3aClick to view benchmark
|
This PR:
BuildableProgram
abstraction, which encapsulates the notion of "some user code that is ready to compile, along with a package store that has all of its dependencies compiled"Visibility
from the AST, since visibility is calculated via exports now.internal
is still parsed without error for backwards compatibility, but is a no-op becauseinternal
is the default now.Visibility::Public
to show up across packages as aGlobalItem
qsharp.json
closes #883
The Q# formatting CI stage will fail until #1692 goes in
This PR supersedes #1693