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

No nvram #617

Merged
merged 4 commits into from
Sep 23, 2020
Merged

No nvram #617

merged 4 commits into from
Sep 23, 2020

Conversation

jreidinger
Copy link
Member

@jreidinger jreidinger commented Sep 22, 2020 •
edited by teclator
Loading

hramrach and others added 3 commits September 22, 2020 10:41
@hramrach
Add UPDATE_NVRAM in /etc/sysconfig/bootloader (bsc#1157550).
cc28a6a 
--removable implies --no-nvram, no need to duplicate it.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
@jreidinger
fix rubocop warning
0ed8135
@jreidinger
fix tests
19196a7
@jreidinger jreidinger mentioned this pull request Sep 22, 2020
Closed
@coveralls
Copy link

coveralls commented Sep 22, 2020 •
edited
Loading

Coverage Status

Coverage increased (+0.1%) to 86.704% when pulling be33461 on no_nvram into dfe082c on master.

@jreidinger
Copy link
Member Author

@watologo1 @hramrach I need your help here. Due to bug checker, we add to master workaround for arm at #615 but I worry its logic collide. As now on arm we run with no-nvram --removable and also another grub2-install without it see https://github.com/yast/yast-bootloader/pull/617/files#diff-a2dd9a79782b903e9a1485d4c37037a7R44

So could you evaluate if we should remove this workaround or if logic is still valid?

@hramrach
Copy link
Contributor

Yes, it will conflict. This needs to be rebased.

@watologo1
Copy link
Contributor

watologo1 commented Sep 22, 2020 via email

@jreidinger
Copy link
Member Author

well, it does not conflict as git conflict, just result is probably not what we want. I fix all travis issues, so please feel free to create pr against this branch and I will handle rest.

@hramrach
Copy link
Contributor

Updated the original PR.

@jreidinger
Copy link
Member Author

@hramrach thanks, I check it and adapt this branch to behave same.

@teclator
Copy link
Contributor

From code point of view it looks good to me.

teclator
teclator approved these changes Sep 23, 2020
View reviewed changes
Copy link
Contributor

@teclator teclator left a comment

Choose a reason for hiding this comment

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

LGTM

teclator
teclator reviewed Sep 23, 2020
View reviewed changes
src/autoyast-rnc/bootloader.rnc
@@ -66,6 +66,7 @@ bl_global =
element os_prober { STRING_BOOL }? &
element suse_btrfs { STRING_BOOL }? &
element secure_boot { STRING_BOOL }? &
element update_nvram { STRING_BOOL }? &
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not forget to update the yast2-schema dependency.

https://github.com/yast/yast-schema/blob/master/package/yast2-schema.spec#L48

@jreidinger jreidinger merged commit 7f9bc8c into master Sep 23, 2020
@jreidinger jreidinger deleted the no_nvram branch September 23, 2020 14:36
@yast-bot
Copy link
Contributor

✔️ Public Jenkins job #65 successfully finished
✔️ Created OBS submit request #836356

@jreidinger jreidinger mentioned this pull request Sep 23, 2020
Merged
@yast-bot
Copy link
Contributor

✔️ Internal Jenkins job #32 successfully finished
✔️ Created IBS submit request #227151

@jreidinger jreidinger mentioned this pull request Sep 23, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teclator teclator teclator approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@jreidinger @coveralls @hramrach @watologo1 @teclator @yast-bot