Re: [PATCH] ZFS/other CoW FS save_env support

[Daniel Kiper]

On Mon, Feb 24, 2020 at 10:30:36AM -0800, Paul Dagnelie wrote:
> On Mon, Feb 24, 2020 at 3:03 AM Daniel Kiper <address@hidden> wrote:
> >
> > Why "root" not "boot"?
> That was a typo on my part; the code uses grub_guess_root_device to
> find the devices backing the default grub directory, but in most
> configurations, this should attempt to locate the boot filesystem
> instead of the root. I was uncertain of a better way to consistently
> determine the boot filesystem, and this portion of the code was copied
> from another GRUB patch
> (https://build.opensuse.org/package/view_file/openSUSE:Factory/grub2/grub2-grubenv-in-btrfs-header.patch?expand=1).
>
> > Yes, please split the patch into smaller patches. Please do one logical
> > change per patch.
> >
> > I quickly went through the patch and pointed things which I spotted at
> > first sight. I will provide more comments when you split the patch into
> > separate patches.
> >
> > Next time please CC following people too: address@hidden,
> > address@hidden and address@hidden.
> Understood! I will post an updated version hopefully today or tomorrow.
>
> >
> > I think that you can drop parenthesis here. And please use NULL instead of 
> > 0.
> Will do. In general, this was one of my questions about writing new
> code in this code base. There are several things where I decided to go
> with consistency with surrounding code instead of what would commonly
> be preferred in modern coding standards or by the style guide (see
> also, the block comment style you mentioned further down). Is there a
> preference in this codebase against consistency when other
> considerations are also relevant?

Please try to be consistent with what you see in a given file. Except
some things which are not inline with grub-dev doc. In these cases stick
to grub-dev.

Daniel