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.

Sign up for GitHub

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

Jump to bottom

Use a custom stylesheet to define CentOS-specific stylesheet data #3477

Merged
merged 1 commit into from Aug 2, 2021
Merged

Use a custom stylesheet to define CentOS-specific stylesheet data #3477

merged 1 commit into from Aug 2, 2021

Conversation

carlwgeorge
Copy link
Contributor

This is a backport of #3179 to the rhel-8 branch. CentOS is trying to take advantage of a custom stylesheet in centos-logos (see rhbz#1974149 and CentOS/centos-logos#2). It's not working and if I understand things correctly this is the change we need to enable it.

@jkonecny12
Copy link
Member

/test

@jkonecny12
Copy link
Member

@poncovka could you please take a look on this PR?

VladimirSlavik
VladimirSlavik approved these changes Jul 1, 2021
View reviewed changes
Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

LGTM, same as original. Testing required of course.

@carlwgeorge carlwgeorge mentioned this pull request Jul 2, 2021
Closed
@carlwgeorge
Copy link
Contributor Author

Just wanted to confirm that building anaconda-33.16.5.2-1.el8 with this PR as a patch resolves CentOS/centos-logos#2.

@poncovka
Copy link
Contributor

poncovka commented Jul 7, 2021

/test

@poncovka
Copy link
Contributor

poncovka commented Jul 7, 2021

/kickstart-test --testtype smoke

@poncovka
Copy link
Contributor

poncovka commented Jul 7, 2021

This solution will start to log an error on RHEL installations, because redhat.css is not provided by redhat-logos, but centos-logos. The error is harmless, but it might be confusing. It would be better to change our centos-stream.conf file instead of rhel.conf to avoid the error message.

redhat_css

@carlwgeorge
Copy link
Contributor Author

Is the same change not going to happen to redhat-logos in 8? If that's the case of course I can change the PR to what you described.

@poncovka
Copy link
Contributor

poncovka commented Jul 8, 2021

Is the same change not going to happen to redhat-logos in 8? If that's the case of course I can change the PR to what you described.

We don't plan to do that in RHEL 8. The overrides for RHEL are part of anaconda-gtk.css and there is no reason to change that.

@carlwgeorge
Use a custom stylesheet to define CentOS-specific stylesheet data
37cbdd6 
Use a stylesheet provided by the centos-logos package.  This is adapted
from rhinstaller#3179, which is intended for RHEL9+, but CentOS Stream 8 is going
to take advantage of that approach as well.

Related: rhbz#1974149
@carlwgeorge
Copy link
Contributor Author

I've reworked this to add custom_stylesheet to the centos-stream.conf file instead.

@carlwgeorge carlwgeorge changed the title Use a custom stylesheet to define RHEL-specific stylesheet data Use a custom stylesheet to define CentOS-specific stylesheet data Jul 9, 2021
@poncovka
Copy link
Contributor

/test

@poncovka
Copy link
Contributor

/kickstart-test --testtype smoke

poncovka
poncovka approved these changes Jul 12, 2021
View reviewed changes
Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

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

Tested. It looks good to me. Thanks!

VladimirSlavik
VladimirSlavik approved these changes Jul 13, 2021
View reviewed changes
Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you!

@poncovka poncovka added the ready to merge The PR can be merged. It should have all BZ flags required for releasing set (usually release+). label Jul 13, 2021
jkonecny12
jkonecny12 approved these changes Jul 19, 2021
View reviewed changes
Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Looks good to me too. Thanks @carlwgeorge !

@rvykydal rvykydal merged commit 54afa21 into rhinstaller:rhel-8 Aug 2, 2021
@carlwgeorge carlwgeorge deleted the rhel-8_backport-3179 branch August 2, 2021 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkonecny12 jkonecny12 jkonecny12 approved these changes

@poncovka poncovka poncovka approved these changes

@VladimirSlavik VladimirSlavik VladimirSlavik approved these changes

Assignees
No one assigned
Labels
ready to merge The PR can be merged. It should have all BZ flags required for releasing set (usually release+). rhel8-branch
Milestone
No milestone
5 participants
@carlwgeorge @jkonecny12 @poncovka @VladimirSlavik @rvykydal