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

Update README.adoc #250

Merged
merged 1 commit into from
Apr 8, 2022
Merged

Conversation

aesteve-rh
Copy link
Member

Signed-off-by: Albert Esteve aesteve@redhat.com

Signed-off-by: Albert Esteve <aesteve@redhat.com>
@sandrobonazzola sandrobonazzola merged commit 2bd32de into oVirt:master Apr 8, 2022
Copy link
Member

@barpavel barpavel left a comment

Choose a reason for hiding this comment

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

@nirs For your attention


- Install additional packages
+
$ sudo dnf install -y $(cat automation/check-patch.packages)
Copy link
Member

@barpavel barpavel Apr 11, 2022

Choose a reason for hiding this comment

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

@nirs I'm not sure changes in lines 160-169 are relevant.
It was an attempt to be similar to PR oVirt/vdsm#121.
There is no automation/check-patch.packages under the project.
Should ioprocess & imageio be here as well?

Copy link
Member

Choose a reason for hiding this comment

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

ioprocess is not relevant for engine host.

Copy link
Member

@barpavel barpavel Apr 12, 2022

Choose a reason for hiding this comment

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

@ahadas @nirs @bennyz
I think the whole Fedora parts can be removed and some other parts updated.
The instructions are about Fedora 30 (!!!)
There is no automation/check-patch.packages.
The https://resources.ovirt.org/pub/ovirt-master-snapshot-static/rpm/ doesn't have a Fedora fc folder at all! Nor it has an el folder, but el8 & el9 instead.
I canl update it :)

Copy link
Member

Choose a reason for hiding this comment

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

@aesteve-rh please remove it in #262

Copy link
Member

Choose a reason for hiding this comment

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

@bennyz Maybe better have a separate "Fedora removal" PR, rather then doing part of the work in the #262 PR, that started as Markdown --> AsciDoc link fixes?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just pushed it as you were commenting this, but I can still separate that commit into another PR if you think that's better. Let's move the dicussion to #262

Copy link
Member

Choose a reason for hiding this comment

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

No need to separate

@barpavel barpavel requested review from barpavel and nirs April 11, 2022 15:57
@aesteve-rh aesteve-rh deleted the patch_readme branch April 12, 2022 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants