grml / grml-rescueboot

33 stars 13 forks source link

Various enhancements #13

Open iskunk opened 1 year ago

iskunk commented 1 year ago

Hello everyone, I recently discovered this project. It is similar to a home-rolled framework I have been using for some time which provides equivalent functionality. I'd like to get rid of the maintenance burden, and switch to using a widely-available project instead. There are a few enhancements I'd like to contribute, however, to provide some functionality I've relied on.

All of the below can be seen in my tree here.

That's everything for now. Before I start submitting pull requests, there are a couple questions I'd like to ask:

  1. What kind of PR structure would you like? One commit with everything, or spread out across multiple commits? (And delineated how?)

  2. The per-ISO config file feature is going to need more documentation. Would it work to have a dedicated section for it in the README, or would breaking it out into a separate README.iso-cfg (?) file be preferable?

Any initial review of the changes would also be appreciated, of course.

mika commented 1 year ago

Hi @iskunk!

This sounds very interesting, haven't had any time yet to look closer into your tree, but some initial feedback from my side:

What kind of PR structure would you like? One commit with everything, or spread out across multiple commits? (And delineated how?)

One commit per main feature tends to be great, so in case of e.g. problems it's usually easier to track down issues (bisecting, reviewing code, reverting stuff,...). Also telling according stories and details in the commit message then tends to be easier, instead of having one single commit with tons of details, if they aren't directly related. So looking at your list of proposed/implemented changes, I'd say combine whatever makes sense in combination. :) Hope I managed to verbalize my basic idea and preferences? :)

The per-ISO config file feature is going to need more documentation. Would it work to have a dedicated section for it in the README, or would breaking it out into a separate README.iso-cfg (?) file be preferable?

I think adding it to the existing debian/README is perfectly fine, especially since it should really be visible as much as possible! (We should also consider adding a main README.md or alike in the main directory so visiting https://github.com/grml/grml-rescueboot is more approachable :))

BTW, https://bugs.debian.org/750072 (#750072: grml-rescueboot: Genericize package) is open since quite some time, maybe we can also use this as opportunity to look into it, just wanted to make sure you're also aware of it.

Your suggestions sound great, very much looking forward to it! :hugs:

iskunk commented 1 year ago

Hope I managed to verbalize my basic idea and preferences? :)

So not all in one commit, but if you're flexible with how the commits are organized, I can try putting together a sequence that makes sense.

I think adding it to the existing debian/README is perfectly fine, especially since it should really be visible as much as possible!

Okay, I'll expand that paragraph into a section.

(We should also consider adding a main README.md or alike in the main directory so visiting https://github.com/grml/grml-rescueboot is more approachable :))

I can put something in for this as well. (It would basically just be a project description and a "see the full README here" kind of thing, as I imagine it.)

BTW, https://bugs.debian.org/750072 (#750072: grml-rescueboot: Genericize package) is open since quite some time, maybe we can also use this as opportunity to look into it, just wanted to make sure you're also aware of it.

Hmm, interesting. So nine years ago, loopback.cfg was already considered a de facto standard. Unfortunate that Ryan's repo hasn't seen any activity since then.

Can I assume that you remain interested in a merge, and perhaps a rename of the project? Ryan continues to be active on here, so we can pull him in for his input once we know which way to go.

Your suggestions sound great, very much looking forward to it! hugs

Happy to get this work in where it can do the most good :-)

iskunk commented 1 year ago

All right, I've reviewed @rfinnie's grub-loopback-iso tree. It's a modified grml-rescueboot rather than a rewrite; here are the salient differences:

The only thing I would do differently is use non-pluralized names /boot/iso/ and /etc/default/grub-loopback-iso, as that is more the Unix style (e.g. we don't have a /usr/share/docs/ directory).

Does all that sound good for me to put into PRs?