Code review comment for lp:~mterry/ubiquity/oem-config-merge

> * UBIQUITY_OEM_USER_CONFIG is a bit verbose as an environment variable name
> (and "OEM user config" isn't a phrase I've ever used in connection with oem-
> config, personally). How about just UBIQUITY_OEM?

So, Ubiquity already had the idea of running in 'oem-config' mode, except just the first part of it (when the OEM is installing the OS and customizing). So I wanted a name that didn't conceptually conflict with the variable already used in that mode (self.oem_config). I figured 'the user configuration portion of an OEM install' somewhat naturally mapped to OEM_USER_CONFIG. I fear UBIQUITY_OEM would be confused with the first portion of an OEM install.

I won't change it yet, but let me know if you're swayed by my golden words above. If not, I can change it. As long as the maintainers know what it means, I suppose it doesn't matter. It's not a user-visible environment variable.

> * Use 1.99 rather than 1.99.0 in configure.ac, so that we don't have to rerun
> autoconf for every minor revision.

OK, will update

> * Since 1.13.8 is currently UNRELEASED, you should merge its changelog
> entries into those for 1.99.0 rather than leaving 1.13.8 as a stub. (Of course
> this will change if 1.13.8 is released before we merge this branch.)

Yeah, I didn't want to mess with that until I actually merged, but I guess it doesn't hurt to be ready. Will update

> * I think you need to rerun debconf-updatepo. I noticed some diffs there that
> seemed to be going backwards ("Erase and use the entire disk" replaced by "Use
> the entire disk").
>
> * There needs to be an oem-config-debconf package, even if it's only a stub;
> finish-install.d/01oem-config-udeb relies on that name, as do existing seeds.
> I think it's a good idea to have that interface anyway.
>
> * I'd rather have netcfg in oem-config than oem-config-debconf (or ubiquity-
> frontend-debconf), even though it's only used in the debconf frontend right
> now; in general I would prefer frontend packages not to deliver component-type
> code as well. (ubiquity-frontend-mythbuntu is a weird exception, perhaps best
> kept that way.)
>
> * Whichever package contains netcfg needs to be Architecture: any, and built
> in *-arch targets in debian/rules; netcfg is written in C.
>
> * Don't hardcode the path to the hostname program.
>
> * set_debconf could be done with a little less duplication, I think - if in
> OEM mode, set dccomm = None and dc = self.db, and refrain from closing dccomm
> at the end. That way the code that actually talks to the debconf instance is
> shared.
>
> * Perhaps OEM-only guards would be useful in the network and tasks
> components? Similarly, I think it would be best to leave them out of
> ubiquity/steps Choices until they actually work properly in ubiquity's
> environment.

OK, will update for the above issues

> * I'm not convinced that we should set OVERRIDE_SYSTEM_USER outside OEM mode;
> the only reason we set that is to cope with the fact that the temporary oem
> user looks like a "system user" (bad naming; it means "ordinary user of the
> system with uid 1000-29999", which has no overlap with 'adduser --system'!).

Hmm, OK. I'll look at it.

> * Why does language_label need an entry in string_questions? I don't see that
> identifier anywhere else.

Uh, right you are. That was from my initial port where I had a "port every quirk from oem-config and pare down later once it's all in place" attitude. But I never pared that line down. Will remove.