Skip to content
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

Backport 2 ⬅️ 5 - Add standalone executables (#121) #256

Draft
wants to merge 1 commit into
base: ign-launch2
Choose a base branch
from

Conversation

Crola1702
Copy link
Contributor

🦟 Bug fix

Fixes #208

Summary

Suggested by #208 (comment) to check if this fixes the problem

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

* Add standalone executables

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Added feedback and more tests

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Add one more test

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Fixed test

Signed-off-by: ahcorde <ahcorde@gmail.com>
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Apr 25, 2024
@claraberendsen
Copy link

@Crola1702 could you rebase and force push with lease with a signed commit? That way the DCO bot won't complain.

#--------------------------------------
# Find ignition-utils
ign_find_package(ignition-utils1 REQUIRED COMPONENTS cli)
set(IGN_UTILS_VER ${ignition-utils1_VERSION_MAJOR})
Copy link
Contributor

Choose a reason for hiding this comment

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

We wouldn't want to add a new dependency to a stable branch. If this is needed to fix the tests, let's just disable them. I'm doing so in #255, so maybe that could be backported instead.

Copy link
Member

Choose a reason for hiding this comment

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

no, it's not for the tests, it's using the cli component for the standalone executables:

I'll see if we can backport some of the cmake changes without switching to a standalone executable

Copy link
Contributor

Choose a reason for hiding this comment

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

@scpeters were you able to take a look to that? let us know if we can help you on that one

@azeey azeey added beta Targeting beta release of upcoming collection and removed beta Targeting beta release of upcoming collection labels Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

6 participants