Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Conversation

@cesarzc
Copy link
Contributor

@cesarzc cesarzc commented Feb 16, 2021

This change implements multiple entry point drivers for different QIR programs.

Copy link
Contributor

@cgranade cgranade left a comment

Choose a reason for hiding this comment

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

Are these samples intended to be user-facing or contributor-facing? If so, we should make sure they are located in repos that onboard to our samples publication pipeline, and that they have all required metadata for said onboarding (see internal wiki for details).

@cesarzc
Copy link
Contributor Author

cesarzc commented Feb 17, 2021

Are these samples intended to be user-facing or contributor-facing? If so, we should make sure they are located in repos that onboard to our samples publication pipeline, and that they have all required metadata for said onboarding (see internal wiki for details).

These samples are contributor-facing. The idea is to have a few examples to know how the generated C++ driver for QIR should look like, so these samples might get replaced or removed at some point when the compiler generates a QIR executable.

Co-authored-by: Chris Granade <chgranad@microsoft.com>
@cesarzc
Copy link
Contributor Author

cesarzc commented Feb 17, 2021

@cgranade for some reason, the reply box is not shown for this message but yes, CLI11.hpp is where CLI::App comes from.

@cesarzc
Copy link
Contributor Author

cesarzc commented Feb 17, 2021

Thank you @irinayat-MS and @cgranade for your comments! I'll address them shortly.

I just wanted to mention that this PR is still work in progress so some things might change considerably.

@kuzminrobin
Copy link
Contributor

I'm done with the first pass of this PR. I didn't dive deep into the code that uses CLI11 library.

I have a single GitHub account tied with my own email at gmail.com (which I check rarely, up to couple of times per day). So if you need my attention to a PR or an issue/bug open on GitHub, then please notify me over Teams or MS email.

@cesarzc cesarzc marked this pull request as ready for review March 8, 2021 16:12
@cesarzc
Copy link
Contributor Author

cesarzc commented Mar 8, 2021

This item seems to me risky, requires resolution. Otherwise seem to me all right to merge.

Thanks for the review Robin! Fixed the issue and it should be ready to merge now.

Copy link
Contributor

@kuzminrobin kuzminrobin left a comment

Choose a reason for hiding this comment

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

Approving with minor recommendations.

@cesarzc cesarzc merged commit 2001b9a into main Mar 9, 2021
@cesarzc cesarzc deleted the cesarzc/prototype-qir-entry-point-driver branch March 9, 2021 17:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants