On 2013-10-21 09:45, Christian PERRIER wrote:
> Please find, for review, the debconf templates and packages descriptions for
> the localepurge source package.
>
> This review will last from Monday, October 21, 2013 to Thursday, October 31,
> 2013.
>
> Please send reviews as unified diffs (diff -u) against the original
> files. Comments about your proposed changes will be appreciated.
>
> Your review should be sent as an answer to this mail.
>
> When appropriate, I will send intermediate requests for review, with
> "[RFRn]" (n>=2) as a subject tag.
>
> When we will reach a consensus, I send a "Last Chance For
> Comments" mail with "[LCFC]" as a subject tag.
>
> Finally, a summary will be sent to the review bug report,
> and a mail will be sent to this list with "[BTS]" as a subject tag.
>
Thank you for the review.
> Rationale:
> --- localepurge.old/debian/localepurge.templates 2013-10-06
> 08:23:05.009303919 +0200
> +++ localepurge/debian/localepurge.templates 2013-10-21 09:27:31.771790999
> +0200
> @@ -1,12 +1,15 @@
> Template: localepurge/nopurge
> Type: multiselect
> Choices: ${locales}
> -_Description: Selecting locale files
> - localepurge will remove all locale files from your system but the ones for
> - the language codes you select now. Usually two character locales like "de"
> - or "pt" rather than "de_DE" or "pt_BR" contain the major portion of
> - localizations. So please select both for best support of your national
> - language settings. The entries from /etc/locale.gen will be preselected
> +_Description: Locale files to keep on this system:
> + The localepurge package will remove all locale files from the system
> + except those that you select here.
> + .
> + It is recommended to choose locales with no country specifier (such
> + as "de", "it, etc.) as well as those that include a country code
> + ("de_DE", "de_CH", "it_IT", etc.).
> + .
> + Entries from /etc/locale.gen will be preselected
> if no prior configuration has been successfully completed.
>
> First of all, we need the synopsis to be a "prompt"....so turning it
> into oe, incuding final colon....
>
I am not familiar with the "oe"-term, could you perhaps explain what it
means?
> Also avoid starting the sentence with "localepurge" to avoid weird
> ccapitalization. Use our usual trick of "The <foo> package"
>
> Split in paragraphs for readability.
>
> Avoid "your system" and be more neutral about whose system this is..:-)
>
Noted.
> Let's try to be clearer about the two kinds of locales (I actually
> wonder if that part is really needed as people who are using the
> package should know what they're doing and know what locales are
> about). Don't use pt_BR as example as this is precisely an exception
> where most localization lies in locales *with* a country specifier..:-))
>
I believe the problem is that localepurge is a bit dumb (I only
"recently" took over the package, so forgive my vagueless here).
Basically, it will keep
/usr/share/locale/$VALUE/
Honestly, I doubt most localepurge-users are aware of the consequences
of only keeping "en_GB.UTF-8" vs "en en_GB en_GB.UTF-8" (without that
warning).
That said, I believe it would be a lot better if localepurge would
detect something like "en_GB.UTF-8" and propose to also keep "en en_GB"
as well.
> Template: localepurge/use-dpkg-feature
> @@ -17,11 +20,11 @@
> from packages being installed.
> .
> Please see /usr/share/doc/localepurge/README.dpkg-path for more
> - information about this feature. It can be enabled (or disabled)
> + information about this feature. It can be enabled (or disabled)
> later by running "dpkg-reconfigure localepurge".
> .
> - Caveat: Changing this option will only take affect for packages
> - unpacked after localepurge has been (re)configured. Packages
> + This option will become active for packages
> + unpacked after localepurge has been (re)configured. Packages
> installed or upgraded together with localepurge may (or may not) be
> subject to the previous configuration of localepurge.
>
> 1st para: drop an extra space. We standardized on *not* using those
> US-style double space after fulls tops.
>
> Drop "caveat". The fact that this is mention on a separate paragraph
> should be enough. I actually wonder whether that warning really
> belongs to a debconf template, particularly because of the "may (or
> may not)" which is anything but clear..:-) (I understand why, but still)
>
Noted.
> @@ -29,40 +32,41 @@
> Type: boolean
> Default: false
> _Description: Really remove all locales?
> - You chose not to keep any locales. This means that all locales will be
> - removed from your system. Are you sure this really is what you want?
> + No locale has been chosen for being kept. This means that all locales will
> be
> + removed from this system. Please confirm whether this is really your
> + intent.
>
> More neutral formulation. Avoid a question in the long description
> (there is already one and that's enough).
>
Ok.
>
> Template: localepurge/remove_no
> Type: note
> -_Description: localepurge will not take any action
> - localepurge will not be useful until you successfully configure it with
> +_Description: No localepurge action until the package is configured
> + The localepurge package will not be useful until you successfully configure
> it with
> the command "dpkg-reconfigure localepurge". The configured entries from
> - /etc/locale.gen of the locales package will then be automagically
> + /etc/locale.gen of the locales package will then be automatically
> preselected.
>
> I don't really understand the purpose of this template and this is a
> debconf note and "Debconf Notes Are Evil" (tm).
>
It is a gem I have inherited. :P It is used if the "above" confirmation
to remove all locales get a negative respons. I suppose it ought to go
back and allow the user to choose something different, but then the user
could get stuck in a loop (if they want to stop and look up something
first).
Maybe I can have it go back to a "menu-ish" thing like d-i... :P
> Still rephrase it to avoid weird capitalization and avoid having a
> sentence as "title".
>
> Also drop automagically which doesn't exist in English (and is IMHO
> abused in most computer documentation).
>
Noted.
>
> Template: localepurge/mandelete
> Type: boolean
> Default: true
> _Description: Also delete localized man pages?
> - Based on the same locale information you chose above, localepurge can also
> - delete superfluous localized man pages.
> + Based on the same locale information you chose, localepurge can also
> + delete localized man pages.
>
> No "above". Depending on the debconf interface, this might be "before"
> and not "above"...:-)
>
> And the localized man pages are not superfluous....they're just not
> used by this system..:-)
>
Ack.
>
> Template: localepurge/dontbothernew
> Type: boolean
> Default: false
> _Description: Inform about new locales?
> - If you are content with the selection of locales you chose to keep and
> - don't want to care about whether to delete or keep newly found locales,
> - just deselect this option to automatically remove new locales you probably
> wouldn't
> - care about anyway. If you select this option, you will be given the
> opportunity
> + If you choose this option, you will be given the opportunity
> to decide whether to keep or delete newly introduced locales.
> + .
> + If you don't choose it, newly introduced locales will be
> + automatically dropped from the system.
>
> Let's make this clearer about what happens when the option is chosen
> or not chosen.
>
>
Seems better indeed.
> Template: localepurge/showfreedspace
> Type: boolean
> Default: true
> _Description: Display freed disk space?
> - localepurge can calculate and display the disk space freed by its
> - operations and show a saved disk space summary at quitting.
> + The localepurge program can calculate and display the disk space freed by
> its
> + operations and show a saved disk space summary when quitting.
>
> Avoid weird capitalization.
>
> "at"? "when"? Justin?
>
Ack on the capitialization, but I don't understand the one-word
questions you asked here.
>
> Template: localepurge/quickndirtycalc
> Type: boolean
> @@ -70,12 +74,12 @@
> _Description: Accurate disk space calculation?
> There are two ways available to calculate freed disk space. One is
> much faster than the other but far less accurate if other changes occur
> - on the filesystem during disc space calculation. The other one works more
> - accurately and is therefore the preferred choice.
> + on the filesystem during disk space calculation. The other one is more
> + accurate but slower.
>
> I usually recommend avoiding "preferred" and referring to the default
> choice in a debconf template. This may have been changed by
> preseeding, for instance.
Okay.
> [...]
>
> --- localepurge.old/debian/control 2013-10-06 08:23:05.009303919 +0200
> +++ localepurge/debian/control 2013-10-21 09:30:14.820172521 +0200
> @@ -11,23 +11,23 @@
> Architecture: all
> Depends: ${misc:Depends}, locales, ucf, debconf | debconf-2.0, procps
> Suggests: debfoster, deborphan, bleachbit
> -Description: Reclaim disk space removing unneeded localizations
> - This is a script to recover disk space wasted for unneeded locales,
> +Description: reclaim disk space by removing unneeded localizations
> + This package provides a script to recover disk space wasted for unneeded
> locales,
> Gnome/KDE localizations and localized man pages. Depending on the
> - installation, it is possible to save some 200, 300, or even more
> - mega bytes of disk space dedicated for localization you will most
> - probably never have any use for. It is run automagically upon
> - completion of any apt installation actions.
> + installation, it is possible to save hundreds of
> + megabytes of disk space dedicated for localization you will most
> + probably never have any use for. It is run automatically upon
> + completion of any APT installation actions.
> .
> - Please note, that this tool is a hack which is *not* integrated with
> - Debian's package management system and therefore is not for the faint
> - of heart. This program interferes with the Debian package management
> + Please note that this tool is a hack which is *not* integrated with
> + the system's package management and therefore is not for the faint
> + of heart. This program interferes with the package management
> and does provoke strange, but usually harmless, behaviour of programs
> related with apt/dpkg like dpkg-repack, reportbug, etc.
> - Responsibility for its usage and possible breakage of your system
> - therefore lies in the sysadmin's (your) hands.
> + Responsibility for its usage and possible breakage of the system
> + therefore lies in the system administrator's hands.
> .
> - Please definitely do abstain from reporting any such bugs blaming
> - localepurge if you break your system by using it. If you don't know
> - what you are doing and can't handle any resulting breakage on your own
> - then please simply don't use this package.
> + Please do abstain from reporting any such bugs blaming
> + localepurge if you break the system by using it. If you don't know
> + what you are doing and can't handle any resulting breakage, you should
> + not install this package.
>
> Synopsis should not start with a capital letter. Adding "by" (Justin?)
>
> Unpersonnalize the package description ("your" system, etc.)
>
Ok
> IMHO, the last paragraph is useless and (even on purpose) too
> frightening....Keeping it anyway as I guess this is your purpose to be
> frightening..:-)
>
>
>
Actually, it was the previous maintainers intention. :) I want to make
localepurge use supported interfaces (see the dpkg-path feature) instead
of its "ad-hoc deletion"-method (which is still the default though).
I will probably delete that paragraph in a (hopefully) not too distant
future. :)
~Niels
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
