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

Add ignition to simplified installer blueprint #389

Merged
merged 3 commits into from
Jun 26, 2024

Conversation

luisarizmendi
Copy link
Contributor

Description

We need to be able to include ignition definition for simplified installer images

Copy link
Collaborator

@ericzolf ericzolf left a comment

Choose a reason for hiding this comment

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

Looks mostly good to me, just wondering about a few things (which might be due to my lack of experience with this project):

  1. shouldn't the new option be documented somewhere?
  2. I don't see any blueprint import file option?

@luisarizmendi
Copy link
Contributor Author

Thanks for reviewing

  1. shouldn't the new option be documented somewhere?
    Well, more than a new option I could say that it was some kind of BUG since you cannot customize the FDO image at all if it's not with ignition that we were filtering out

  2. I don't see any blueprint import file option?
    Sorry this comes from a previous PR where I forgot to increase the galaxy version, so the content was not "updated" in galaxy. This is the PR already accepted: Include blueprint import file option #357

@ericzolf
Copy link
Collaborator

Then approved!

@luisarizmendi
Copy link
Contributor Author

@ericzolf Azure pipelines are failing and blocking the merge, but I suppose that's not related to the changes introduced in this PR, am I right?

@mergify mergify bot closed this Jun 24, 2024
Copy link
Contributor

mergify bot commented Jun 24, 2024

This Pull Request looks stale. Feel free to reopen it if you think it's a mistake.

@luisarizmendi
Copy link
Contributor Author

@ericzolf @maxamillion can you please look at why the PR was not merged after approval? Do I need to change anything?

Thanks.

@ericzolf ericzolf reopened this Jun 26, 2024
@ericzolf ericzolf self-requested a review June 26, 2024 09:18
Copy link
Collaborator

@ericzolf ericzolf left a comment

Choose a reason for hiding this comment

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

I thought I had already done it but let me approve (again).

@ericzolf
Copy link
Collaborator

The short answer is that I don't know, I thought I had approved but perhaps I haven't... Also I don't know if you see but many checks were not successful, though I doubt due to your changes.

@ericzolf ericzolf merged commit d3031d5 into redhat-cop:main Jun 26, 2024
11 of 76 checks passed
Copy link
Contributor

mergify bot commented Jun 27, 2024

This Pull Request needs all checks to run successfully. Could you fix it @luisarizmendi? 🙏

@mergify mergify bot added the invalid This doesn't seem right (PR check failure?) label Jun 27, 2024
Copy link
Contributor

mergify bot commented Jun 28, 2024

This Pull Request needs all checks to run successfully. Could you fix it @luisarizmendi? 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right (PR check failure?)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants