Bug 488995
| Summary: | Review Request: pidgin-tlen - Tlen IM Pidgin plugin | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Dominik 'Rathann' Mierzejewski <dominik> |
| Component: | Package Review | Assignee: | Simon <cassmodiah> |
| Status: | CLOSED NOTABUG | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | cassmodiah, christoph.wickert, fedora-package-review, notting |
| Target Milestone: | --- | Keywords: | Reopened |
| Target Release: | --- | Flags: | dominik:
fedora-review?
|
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2009-12-19 21:28:42 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Dominik 'Rathann' Mierzejewski
2009-03-06 17:07:30 UTC
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.
some issues:
--
%global snap 20090209
why global instead of define?
--
%{__make}
please use commands like they are, not as a macro.
make instead of %{__make}
--
BuildRoot Tag
https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
your buildroot tag is not valid!
--
%defattr(644,root,root,755)
please use %defattr(-,root,root,-)
--
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.
--
BuildRequires: pidgin-devel
Requires: pidgin
if you have as BR pidgin-devel, then will rpm requires pidgin automaticly.
you don't need Requires: pidgin
--
this is a tlen plugin, so it will be need
Requires: libtlen
or?! I didn't test it, yet.
(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. > 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. https://fedoraproject.org/wiki/Packaging:GuidelinesTodo should be okay. >> please use commands like they are, not as a macro. >> make instead of %{__make} > OK, but what's wrong with using macros? Nothing, it's just a sugestion. I would prefer make, so I tried to convince you. If you don't like it, you can use macros of course, if you want. >> %defattr(644,root,root,755) >> please use %defattr(-,root,root,-) > Why? This is the part you should answer why you are not using %defattr(-,root,root,-) https://fedoraproject.org/wiki/Packaging/Guidelines#File_Permissions If you have a good reason to differ from default, I will have no problem with this. > You're picky, but OK. I'm definitly not picky. :-) > IIRC it won't, because plugins are dlopen()'d, but I'll re-check. yeah i was wrong, sorry! my fault!!! > 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. That puts another complexion on the matter. https://fedoraproject.org/wiki/Packaging/Guidelines#Duplication_of_system_libraries If you decide to patch it out, please comment your patch(es) https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment (In reply to comment #2) > (In reply to comment #1) > > %defattr(644,root,root,755) > > please use %defattr(-,root,root,-) > > Why? Because it's both in the packaging and the review guidelines. http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions > > 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. No he's not, it's also part of the guidelines: http://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS (In reply to comment #3) > >> please use commands like they are, not as a macro. > >> make instead of %{__make} > > OK, but what's wrong with using macros? > Nothing, it's just a sugestion. I would prefer make, so I tried to convince > you. If you don't like it, you can use macros of course, if you want. Changed. > >> %defattr(644,root,root,755) > >> please use %defattr(-,root,root,-) > > Why? > This is the part you should answer why you are not using > %defattr(-,root,root,-) > https://fedoraproject.org/wiki/Packaging/Guidelines#File_Permissions > If you have a good reason to differ from default, I will have no problem with > this. I don't. Changed. > > 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. > That puts another complexion on the matter. > https://fedoraproject.org/wiki/Packaging/Guidelines#Duplication_of_system_libraries It's not duplication, because the code is different (only auth.c is taken from an older release of libtlen). Anyway, I've sent upstream an email asking about libtlen usage. http://rathann.fedorapeople.org/review/pidgin-tlen.spec http://rathann.fedorapeople.org/review/pidgin-tlen-0-0.2.20090209.fc10.src.rpm https://bugzilla.redhat.com/show_bug.cgi?id=490140 https://bugzilla.redhat.com/show_bug.cgi?id=495310 I will wait, because this topic is hot! 2 months without response. i will close this. please feel free to reopen it again, without a local copy of libtlen (In reply to comment #7) > 2 months without response. Yeah, sorry about that. I got tangled up in real-life. > i will close this. please feel free to reopen it again, without a local copy of > libtlen I got a reply from upstream. They said they use only a some parts of libtlen and that it's not feasible to use the external library. okay, I noted that before. fact is, you can't use an internal, even it is forked... Ask upstream to create a tarball of the forked libtlen, pacakge it, open a review request, import it to fedora and build your plugin against it. this won't be a problem.. If upstream don't create a tarball. You can do it and upload it to your fedorapeople space and perhaps a little html document with some descriptions or so (for URL-tag ins pec, a directory listing isn't a good page. people like content). then you can do the steps i discribed... (In reply to comment #10) > Ask upstream to create a tarball of the forked libtlen, pacakge it, open a > review request, import it to fedora and build your plugin against it. > this won't be a problem.. There's no "forked libtlen" in this plugin. Upstream has simply taken some code (a header file and some functions IIRC) from libtlen, nothing more. > If upstream don't create a tarball. You can do it and upload it to your > fedorapeople space and perhaps a little html document with some descriptions or > so (for URL-tag ins pec, a directory listing isn't a good page. people like > content). then you can do the steps i discribed... I could probably try to port this plugin to use external libtlen, but I don't have time for that right now. |