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

Grub config fix #2653

Merged
merged 3 commits into from
May 31, 2021
Merged

Grub config fix #2653

merged 3 commits into from
May 31, 2021

Conversation

watologo1
Copy link
Contributor

No description provided.

@watologo1 watologo1 requested review from nodeg and SchoolGuy May 29, 2021 17:17
@watologo1
Copy link
Contributor Author

Hi Enno.
bootconfig is written again.
This might also help for the images part Dominik was looking at.

You introduced a bug while pleasing the shell script linter. The quotations must not be added or the code will not work.
No idea how to except this for the linter. To be honest, we do not have much shell scripts, a lint_shellscripts looks somewhat overdosed...

I realized, that the default boot entry still is local, even it should be switched to the chosen profile after doing --enable-netboot
I'll also have a look at this the next days, but this is not that sever anymore...

@SchoolGuy SchoolGuy added this to Pull Requests in Backlog via automation May 30, 2021
@SchoolGuy SchoolGuy self-assigned this May 30, 2021
@SchoolGuy SchoolGuy added this to the v3.3.0 milestone May 30, 2021
@SchoolGuy SchoolGuy added the main Not a release but referring to the Git main branch label May 30, 2021
SchoolGuy
SchoolGuy reviewed May 30, 2021
View reviewed changes
cobbler/tftpgen.py Outdated Show resolved Hide resolved
@watologo1
Consolidate update_netboot() and add_single_system() to sync_single_s…
83432d2 
…ystem()
@watologo1
Always write menu entries, also in grub case
93447e2 
And consolidate functions:
sync_single_system -> get_menu_items if not provided and make
use of sync_singel_system in sync_systems and others
@watologo1
Remove quotation marks
392d40a 
grub modules must be added as individual params, not as one string param
to grub2-mkimage
@codecov
Copy link

codecov bot commented May 31, 2021

Codecov Report

Merging #2653 (392d40a) into master (ef4585c) will increase coverage by 0.01%.
The diff coverage is 6.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2653      +/-   ##
==========================================
+ Coverage   27.53%   27.55%   +0.01%     
==========================================
  Files          86       86              
  Lines       12405    12413       +8     
==========================================
+ Hits         3416     3420       +4     
- Misses       8989     8993       +4
Impacted Files Coverage Δ
cobbler/actions/sync.py 25.71% <0.00%> (-0.32%) ⬇️
cobbler/modules/managers/in_tftpd.py 31.57% <10.00%> (+3.39%) ⬆️
cobbler/cobbler_collections/manager.py 67.32% <0.00%> (-0.95%) ⬇️
cobbler/items/package.py 54.54% <0.00%> (-0.46%) ⬇️
cobbler/items/item.py 35.13% <0.00%> (-0.30%) ⬇️
cobbler/utils.py 55.84% <0.00%> (-0.10%) ⬇️
cobbler/items/system.py 21.85% <0.00%> (-0.05%) ⬇️
cobbler/tftpgen.py 5.61% <0.00%> (-0.03%) ⬇️
cobbler/actions/buildiso.py 3.40% <0.00%> (-0.02%) ⬇️
cobbler/cobbler_collections/systems.py 36.11% <0.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef4585c...392d40a. Read the comment docs.

@watologo1 watologo1 requested a review from SchoolGuy May 31, 2021 07:33
SchoolGuy
SchoolGuy approved these changes May 31, 2021
View reviewed changes
Copy link
Member

@SchoolGuy SchoolGuy left a comment

Choose a reason for hiding this comment

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

LGTM in my eyes now.

@SchoolGuy SchoolGuy assigned nodeg and unassigned SchoolGuy May 31, 2021
@SchoolGuy
Copy link
Member

@nodeg If you find this okay too then please add the checkmark and merge afterwards. :)

nodeg
nodeg approved these changes May 31, 2021
View reviewed changes
Copy link
Member

@nodeg nodeg left a comment

Choose a reason for hiding this comment

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

LGTM, although I did not test it explicitly. I am relying here on @watologo1.

@nodeg nodeg merged commit 68225e2 into cobbler:master May 31, 2021
Backlog automation moved this from Pull Requests to Done May 31, 2021
@SchoolGuy SchoolGuy removed this from Done in Backlog Jun 29, 2021
@SchoolGuy SchoolGuy added this to Pull Requests in V3.3.0 via automation Jun 29, 2021
@SchoolGuy SchoolGuy moved this from Pull Requests to Done in V3.3.0 Jun 29, 2021
@SchoolGuy SchoolGuy deleted the grub_config_fix branch July 30, 2021 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nodeg nodeg nodeg approved these changes

@SchoolGuy SchoolGuy SchoolGuy approved these changes

Assignees

@nodeg nodeg

Labels
main Not a release but referring to the Git main branch
Projects
No open projects
V3.3.0
Done
Milestone
v3.3.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@watologo1 @SchoolGuy @nodeg