Details
- Reviewers
emaste bapt khorben_defora.org - Group Reviewers
pkgbase
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 63629 Build 60513: arc lint + arc unit
Event Timeline
| usr.sbin/bsdinstall/scripts/auto | ||
|---|---|---|
| 169 | I personally didn't feel the need to as the function would only be called in this one place. I don't have a strong opinion though and don't see a reason we couldn't put everything in this else branch in a function. If you'd prefer that say the word and I'll change it :) | |
| usr.sbin/bsdinstall/scripts/auto | ||
|---|---|---|
| 156–160 | Titles and button labels should all be Title Case | |
| 158 | The wording here should match what's used elsewhere for pkgbase. I also don't know how much we should be signposting things here; do we want "If unsure, select standard"? Also it shouldn't just be the button label that indicates pkgbase is experimental. | |
| usr.sbin/bsdinstall/scripts/auto | ||
|---|---|---|
| 158 | I've reworded this prompt in the latest update to this patch, let me know what you think. I don't think there are well established conventions for pkgbase related terminology in src yet. This patch stack introduces some of the first user-facing mentions of pkgbase. | |
| usr.sbin/bsdinstall/scripts/auto | ||
|---|---|---|
| 158 | I've suggested alternate wording in the jail patch that I think is a bit more future-proof. | |
| 169 | My thought is how much of this is shared with the jail script, and if it would make sense to have a 'distbase' bsdinstall command that this and the jail command share similar to the sharing you are now doing with pkgbase. If it is a big pain it's probably not worth it, but it would be a lot cleaner and make this set of changes easier to read and review overall if it is feasible. | |
| usr.sbin/bsdinstall/scripts/auto | ||
|---|---|---|
| 169 | I did try extracting this "distbase" logic to a separate target originally but it didn't seem worthwhile. The only part that seemed possible to move to a separate target without significant refactoring was the "Distribution Select" dialog. The logic in the auto and jail targets is actually more different than it might seem at first glance. The jail script does not use the fetchmissingdists target for example. The auto script also splits the logic into separate select and fetch/extract/install steps with other operations (disk formatting) in between while the jail script does not. Anyhow, I don't think it's worth risking a regression to do heavy refactoring of this logic right now. | |