Skip to content

🎨 Install All Header into Device#325

Open
ystade wants to merge 12 commits intodevelopfrom
install-all-header
Open

🎨 Install All Header into Device#325
ystade wants to merge 12 commits intodevelopfrom
install-all-header

Conversation

@ystade
Copy link
Collaborator

@ystade ystade commented Feb 11, 2026

Description

Only two of the three headers necessary for the device interface are prefixed. The constants.h is not. However, only due to this fact, every device depends on an available QDMI installation. This PR also adds the constants.h header to the files that are copied into the include directory of the device.

Checklist:

  • The pull request only contains commits that are focused and relevant to this change.
  • I have added appropriate tests that cover the new/changed functionality.
  • I have updated the documentation to reflect these changes.
  • I have added entries to the changelog for any noteworthy additions, changes, fixes, or
    removals.
  • I have added migration instructions to the upgrade guide (if needed).
  • The changes follow the project's style guidelines and introduce no new warnings.
  • The changes are fully tested and pass the CI checks.
  • I have reviewed my own code changes.

@ystade ystade added this to the v1.3.0 milestone Feb 11, 2026
@ystade ystade self-assigned this Feb 11, 2026
@ystade ystade added enhancement Enhancement of existing functionality refactor Refactoring the code base dependencies Updates/Removals/Additions regarding dependencies usability Increasing usability of the library labels Feb 11, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 11, 2026

PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://Munich-Quantum-Software-Stack.github.io/QDMI/pr-preview/pr-325/

Built to branch gh-pages at 2026-02-12 07:08 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@codecov
Copy link

codecov bot commented Feb 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Impacted file tree graph

@@           Coverage Diff           @@
##           develop    #325   +/-   ##
=======================================
  Coverage     90.3%   90.3%           
=======================================
  Files            6       6           
  Lines          732     732           
  Branches       141     141           
=======================================
  Hits           661     661           
  Misses          71      71           
Flag Coverage Δ
cpp 90.3% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ystade
Copy link
Collaborator Author

ystade commented Feb 11, 2026

The coverage is concerning. It probably dropped because the constants header is not taken from the include directory in the device implementation anymore. However, I do not see how to fix it. In particular, the very same header should still be used by the driver directly.

@ystade ystade requested a review from burgholzer February 11, 2026 09:23
Copy link
Contributor

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

I would assume that the template itself also needs an update, but you are probably covering this in #326 (have not checked so far).

If this is supposed to go into a 1.2.2 release of QDMI, this change also needs to be back ported to the v1.2.x branch.
If this should, instead, constitute a v1.3.0 release, no back port is necessary.
(Note that we do not currently have mergifyio set up like in mqt-core, which would make this a bit easier).

Copy link
Contributor

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

Ah, the reason coverage is missing is because the QDMI library is no longer linked to the example device so the respective coverage settings are no longer propagated to the target. This needs to be fixed.

@ystade
Copy link
Collaborator Author

ystade commented Feb 12, 2026

@burgholzer I adopted the cmake config quite a bit to enable coverage for devices. Kindly have a look, whether this is all fine.

@ystade ystade requested a review from burgholzer February 12, 2026 07:09
@ystade
Copy link
Collaborator Author

ystade commented Feb 12, 2026

I would assume that the template itself also needs an update, but you are probably covering this in #326 (have not checked so far).

If this is supposed to go into a 1.2.2 release of QDMI, this change also needs to be back ported to the v1.2.x branch. If this should, instead, constitute a v1.3.0 release, no back port is necessary. (Note that we do not currently have mergifyio set up like in mqt-core, which would make this a bit easier).

  1. Yes, the template must also be changed. It is easier to do that in 🧑‍💻 Base Example Device on Template #326 because otherwise the "Build and Install" job fails due to missing header files, as this refers to the develop branch. Hence, it is cleaner to merg this PR and adapt the template in a subsequent PR.

  2. I would be fine to let that constitute a v1.3.0 or maybe a v1.2.3 release. Not needing to link a target anymore warrants a new path/minor version in my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Updates/Removals/Additions regarding dependencies enhancement Enhancement of existing functionality refactor Refactoring the code base usability Increasing usability of the library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants