[Bug 488995] Review Request: pidgin-tlen - Tlen IM Pidgin plugin

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=488995





--- Comment #2 from Dominik 'Rathann' Mierzejewski <rpm at greysector.net>  2009-03-27 05:29:33 EDT ---
(In reply to comment #1)
> I didn't find your surname in fas. You need a sponsor, right?
> I will add the need sponsor blocker, if i am wrong, then please remove it.

As you've already found out, I don't need a sponsor.

> some issues:
> --
> %global snap 20090209
> why global instead of define?

https://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define

And it's been ratified by FESCo already.

Although in this particular case it doesn't make a difference.

> --
> %{__make}
> please use commands like they are, not as a macro.
> make instead of %{__make}

OK, but what's wrong with using macros?

> --
> BuildRoot Tag
> https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
> your buildroot tag is not valid!

OK.

> --
> %defattr(644,root,root,755)
> please use %defattr(-,root,root,-)

Why?

> --
> makro couples 
> please use:
> $RPM_OPT_FLAGS and $RPM_BUILD_ROOT
> or
> %{buildroot} and %{optflags}
> please use one of this couples, but do not mix this.

You're picky, but OK.

> --
> BuildRequires: pidgin-devel
> Requires: pidgin
> if you have as BR pidgin-devel, then will rpm requires pidgin automaticly.
> you don't need Requires: pidgin

IIRC it won't, because plugins are dlopen()'d, but I'll re-check.

> --
> this is a tlen plugin, so it will be need 
> Requires: libtlen
> or?! I didn't test it, yet.  

No, it has its own implementation. I'm not sure if it makes sense to ask
upstream to use libtlen because libtlen hasn't been updated in quite a while.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.