[Please use X-debbugs-cc: rather than Cc: when you want a new bug to be copied to someone else. I had to look up the bug number in order to be able to reply.]
Hello Paul,
Thank you for your interest. Here is a review of the patch.
On Sat 05 Jan 2019 at 09:19am -0800, Paul Hardy wrote:
> In git-debrebase.5.pod, after "reified" I added " [re-quiltified?]" as
> a guess of what you meant. You'll want to change that accordingly.
I think we just meant 'reified' ... I'm afraid no synonyms spring to
mind.
> -You should also write appropriate dput configuration,
> +You should also write an appropriate dput configuration,
Either "appropriate dput configuration" or "an appropriate dgit
configuration file", but not "an appropriate dput configuration", I
think.
> dgit clone needs to be told the source package name
> -(which might be different to the binary package name,
> +(which might be different from the binary package name,
Aren't either acceptable in contemporary English? ('to' is what I would
expect myself to write)
> -different to or even disjoint from dgit's view.
> +different from or even disjoint from dgit's view.
Similarly.
> the .dsc. It then pushes the HEAD to the suite's dgit-repos branch,
> adjusts the .changes to include any .origs which the archive lacks
> -and exclude .origs which the archive has
> +and excludes .origs which the archive has
This is wrong -- it should agree with the 'include' in the previous line.
> @@ -355,7 +355,7 @@ in .git/info/attributes,
> but it is insufficient,
> because it was made by an earlier version of dgit
> and git has since introduced new transforming attributes,
> -modifies the macro to disable the newer transformations.
> +this modifies the macro to disable the newer transformations.
I think the comma just before your change now needs to be a semicolon.
> -the latter unless the former exists on PATH.)
> +the latter unless the former exists in PATH.)
This is wrong. "in PATH" suggests actually present in the string PATH;
"on PATH" means in one of the directories specified by PATH, AFAIK.
> .BR dgit-distro. \fIdistro\fR .clean-mode-newer
> Like .clean-mode,
> -but ignored if the value does not make sense to this version of dgit.
> +but ignored if the value does not make sense for this version of dgit.
I think this is wrong. Perhaps s/does not make sense to/is unknown to/.
> -Whether to setup a merge driver which uses dpkg-mergechangelogs for
> +Whether to set up a merge driver which uses dpkg-mergechangelogs for
I think this is wrong. AIUI, 'setup' is a noun and 'set up' is a verb.
> -It should only be used with a care and
> +It should only be used with care and
"It should be used only with care"*
:)
> @@ -135,7 +135,7 @@ See L</STITCHING, PSEUDO-MERGES, FFQ RECORD>.
>
> * Maintainer's HEAD was here while they were editing,
> before they said they were done, at which point their
> - tools made -/- (and maybe %) to convert to
> + tools made -/- (and maybe %) convert to
> the fast-forwarding interchange branch.
This is wrong. It changes the meaning to something other than what was
intended.
Otherwise, LGTM.
--
Sean Whitton
signature.asc
Description: PGP signature
