control: tag -1 -moreinfo On 2017-07-15 02:28, Sean Whitton wrote: > > Here is a review of 534d41f:
Thank you for reviewing it. > - I think we should list Dmitry in the Uploaders: field Done > - your git history does not really give credit to Dmitry for his work. > I'd like to suggest starting again, and doing it like this: This is because I didn't start using dgit from the beginning, and I rebuilt debian/ in a separate working tree. Now I better understand dgit's philosophy and I started over. The repository is still at https://anonscm.debian.org/cgit/users/paride-guest/farbfeld.git/ but it's a new one, which retains the full history. > - I also think it would be good to state in debian/changelog that most > of the Debianisation is due to Dmitry Done > - your changes to the patch header do not make sense: the '3..' will not > yield "the changes made by the Debian maintainer in the first upload > of upstream version 3". Should be fixed now. The 'debian/3-1' tag is still missing, I guess the right time to tag it is after the package is accepted. > - I disagree with you about Dmitry's `convert` patch. It just doesn't > seem likely to me that there would be difficult merge conflicts with > new upstream versions, and it is indeed useful to inform the user that > convert is not available. But I will defer to your judgement -- if > you're sure about dropping the patch, maybe imagemagick should be > moved to a hard dependency? I still believe this patch belongs to upstream, and even if it's trivial to maintain it already prevented a clean 'git merge' of upstream version 3. The package works fine even without imagemagick, it just can't handle all those image formats. Imagemagick itself is not very kind when a helper binary is missing: $ convert test.jpg test.webp convert-im6.q16: delegate failed `'cwebp' -quiet %Q '%i' -o '%o'' @ error/delegate.c/InvokeDelegate/1919. (cwebp is provided by the webp package, which is not even suggested by imagemagick). In general I believe it's better not to apply patches whenever possible, for several reasons. I understand this one is trivial, but the issue it addresses is, in my opinion, even more trivial. This said, I left it in. I explained my general thought on the topic, but I value your feedback and while I think there is no strong reason to include this patch, I see there is no strong reason to oppose it. I would be against a hard dependency on imagemagick. Thanks again, Paride
