Page MenuHomeFreeBSD

bsdinstall: Provide help text for partitioning options
ClosedPublic

Authored by woodsb02 on Jan 17 2020, 7:12 AM.

Details

Summary

bsdinstall: Provide help text for partitioning options

Includes commentary of when ZFS or UFS are recommended.

Also hoist the options text to the top of script as variables
(will help with future international translations).

Test Plan

Test run bsdinstall - works ok

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

woodsb02 created this revision.Jan 17 2020, 7:12 AM
woodsb02 updated this revision to Diff 66910.Jan 17 2020, 1:53 PM

Use dialog menu tag variables in subsequent case statement
(rather than re-typing the variable contents)

philip accepted this revision.Jan 18 2020, 12:29 AM

Looks good to me!

This revision is now accepted and ready to land.Jan 18 2020, 12:29 AM
cem resigned from this revision.Jan 18 2020, 1:09 AM
woodsb02 updated this revision to Diff 67272.Sat, Jan 25, 3:58 AM

Modify help text inline with discussion on freebsd-arch@ mailing list.

This revision now requires review to proceed.Sat, Jan 25, 3:58 AM
philip accepted this revision.Sat, Jan 25, 4:05 AM

Looks good. I like the pointer to the tuning guide.

This revision is now accepted and ready to land.Sat, Jan 25, 4:05 AM
woodsb02 updated this revision to Diff 67273.Sat, Jan 25, 4:08 AM

msg_partitioning_ufs_help: Update text to avoid implying multiple disks can be selected

This revision now requires review to proceed.Sat, Jan 25, 4:08 AM
philip accepted this revision.Sat, Jan 25, 4:09 AM
This revision is now accepted and ready to land.Sat, Jan 25, 4:09 AM
rgrimes accepted this revision.Sat, Jan 25, 4:23 PM
rgrimes added a subscriber: rgrimes.

Other than perhaps a uniformity issue, that you can ignore if you wish, this looks fine to me.

usr.sbin/bsdinstall/scripts/auto
56 ↗(On Diff #67273)

To be more uniform could this be "Guided UFS Disk Setup" as the zfs desc above explicity mentions ZFS too.

dteske requested changes to this revision.Sun, Jan 26, 3:24 AM
dteske added inline comments.
usr.sbin/bsdinstall/scripts/auto
52–63 ↗(On Diff #67273)

The variable names here need to be adjusted.

msg_partititoning_zfs="Auto (ZFS)"

For example should be:

msg_auto_zfs="Auto (ZFS)"

So that when used in code, the code reads like the text. Also, when searching the code, one can prepend "msg_", change to lower-case, and replace non-printable characters and spaces into underscore to find uses of the text.

Also, should the text be re-usable elsewhere, it is not tied to the partitioning menu. This is so that when someone performs international translation, fewer translations have to be made. For example, there is no reason to have:

msg_partitioning_shell="Shell"

When the following will do:

msg_shell="Shell"

Which makes any singular occurrence of the word "Shell" accessible under the "msg_shell" variable. Unnecessarily localizing individual words and phrases to a particular dialog, function, or module creates duplication and extra work for translators.

This revision now requires changes to proceed.Sun, Jan 26, 3:24 AM
woodsb02 updated this revision to Diff 67352.Mon, Jan 27, 1:30 PM
woodsb02 marked 2 inline comments as done.
  • Mention "UFS" in description of "Auto (UFS)" option (recommended by rgrimes)
  • Adjust variable names to read like the text (recommended by dteske)
  • Sort msg_ variable definitions in alphabetical order (as per scripts/zfsboot)

Thanks Rod and Devin for your comments - I have incorporated them

rgrimes accepted this revision.Mon, Jan 27, 6:01 PM
This revision was not accepted when it landed; it landed in state Needs Review.Mon, Jan 27, 10:46 PM
This revision was automatically updated to reflect the committed changes.