Discussion:
Bug#851937: RFS: farbfeld/2.20170109-1 ITP
(too old to reply)
Paride Legovini
2017-07-12 13:10:02 UTC
Permalink
Raw Message
Following up from the brief discussion we had on #debian-devel, here is
a tentative package:

https://anonscm.debian.org/cgit/users/paride-guest/farbfeld.git/

I adopted the dgit-maint-merge(7) workflow as Dmitry done initially.
After cloning the repository the package can be built like this:

$ cd farbfeld
$ git deborig
$ dgit build -tc

Lintian seems fairly clean on the output package.

Note that I didn't bring in this patch:

https://anonscm.debian.org/cgit/users/kaction-guest/farbfeld.git/commit/?id=16c1e8ac96df9e81eec9c6eb83f05ca27fac47c2

as I think the Recommends: it enough, and in general I prefer not to
diverge from upstream whenever possible. As always, comments are welcome.

Cheers,

Paride
Sean Whitton
2017-07-14 15:10:01 UTC
Permalink
Raw Message
Dear Paride,
I'm not sure what's the best practice here, so before doing any further
work I'll wait for your opinion.
Somehow this e-mail didn't reach me, so sorry for not replying.
Following up from the brief discussion we had on #debian-devel, here is
https://anonscm.debian.org/cgit/users/paride-guest/farbfeld.git/
I adopted the dgit-maint-merge(7) workflow as Dmitry done initially.
$ cd farbfeld
$ git deborig
$ dgit build -tc
Thanks. Will review & hopefully upload soon.
--
Sean Whitton
Sean Whitton
2017-07-15 00:40:01 UTC
Permalink
Raw Message
control: tag -1 +moreinfo
Post by Paride Legovini
Following up from the brief discussion we had on #debian-devel, here is
https://anonscm.debian.org/cgit/users/paride-guest/farbfeld.git/
Thanks again for your help with this RFS. Here is a review of 534d41f:

- I think we should list Dmitry in the Uploaders: field, which would
indicate that he may upload new versions of the package without it
counting as an NMU

- 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:

+ clone Dmitry's repo
+ `git merge 3` to get the new upstream version
+ revert the convert commit (but see below)
+ apply your other changes

- I also think it would be good to state in debian/changelog that most
of the Debianisation is due to Dmitry

- 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". Please take another look at the template.

- 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?

If you're able to address the issues I've raised in this message, please
remove the moreinfo tag in this bug, and don't forget to re-run `dch -r`
to refresh the changelog timestamp.
--
Sean Whitton
Paride Legovini
2017-07-17 07:50:02 UTC
Permalink
Raw Message
control: tag -1 -moreinfo
Thank you for reviewing it.
Post by Sean Whitton
- I think we should list Dmitry in the Uploaders: field
Done
Post by Sean Whitton
- your git history does not really give credit to Dmitry for his work.
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.
Post by Sean Whitton
- I also think it would be good to state in debian/changelog that most
of the Debianisation is due to Dmitry
Done
Post by Sean Whitton
- 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.
Post by Sean Whitton
- 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

Loading...