Bug 222257 - Review Request: pastebin - A collaborative debugging tool
Summary: Review Request: pastebin - A collaborative debugging tool
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dennis Gilmore
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Keywords:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2007-01-11 05:41 UTC by Michael Stahnke
Modified: 2007-11-30 22:11 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-01-12 21:29:06 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

Description Michael Stahnke 2007-01-11 05:41:30 UTC
Spec URL: http://www.stahnkage.com/rpms/pastebin.spec
SRPM URL: http://www.stahnkage.com/rpms/pastebin-0.50-1.src.rpm
Description: pastebin is here to help you collaborate on debugging code snippets. 
If you're not familiar with the idea, most people use it to submit a code
fragment to pastebin, getting a url like http://pastebin.com/1234 and then
link that URL in IRC or IM conversations.  This allows others to see your
code and optionally post changes.

Comment 1 Dennis Gilmore 2007-01-11 06:21:43 UTC
 package meets naming and packaging guidelines.
 specfile is properly named, is cleanly written and uses macros consistently.
 dist tag is present.
 build root is correct.
      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 license field matches the actual license.
 license is open source-compatible.  GPL License text included in package.
 source files match upstream:
      d7b8993f4baed7753fb7c912b06725fb  pastebin.tar.gz
      d7b8993f4baed7753fb7c912b06725fb  ../SOURCES/pastebin.tar.gz
 latest version is being packaged.
 BuildRequires are proper.
 package builds in mock ( FC-6 ).
 rpmlint is silent.
 final provides and requires are sane
 no shared libraries are present.
 package is not relocatable.
 doesn't own any directories it shouldn't.
 file permissions are appropriate.
 %clean is present.
 no scriptlets present.
 code, not content.
 documentation is small, so no -docs subpackage is necessary.
 %docs are not necessary for the proper functioning of the package.
 no headers.
 no pkgconfig files.
 no libtool .la droppings.
 not a GUI app.

Needs fixing 
does not own %{_datadir}/%{name}
%{_sysconfdir}/%{name} listed twice


Comment 2 Michael Stahnke 2007-01-11 15:31:41 UTC
I have fixed he above issues. 

Spec URL: http://www.stahnkage.com/rpms/pastebin.spec
SRPM URL: http://www.stahnkage.com/rpms/pastebin-0.50-2.src.rpm

Comment 3 Dennis Gilmore 2007-01-11 15:41:42 UTC
ok looks good and the package works.

APPROVED

and ill sponsor you also  as it seems you are not sponsored already 

apply for cvsextras group access

Comment 4 Michał Bentkowski 2007-01-11 16:08:32 UTC
(In reply to comment #1)
>  rpmlint is silent.

rpmlint is not silent:
W: pastebin mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 12)

Also, there are some documentation files in %{_datadir}/pastebin/lib/geshi/docs
which should be moved to %doc. Creation of new directory there, like geshi
sounds like a good solution. It seems to me that the content of 
{_datadir}/pastebin/lib/geshi may be moved there as well, obviously after some
fixing in example.php.

Comment 5 Michał Bentkowski 2007-01-11 16:10:55 UTC
Oops, I didn't want to set an FE-REVIEW blocker back. Changing to FE-ACCEPT
again.

Comment 6 Michael Stahnke 2007-01-11 16:27:19 UTC
Are you saying the geshi doc should be in /usr/share/doc/geshi or
/usr/share/doc/pastebin-0.50/geshi?


Comment 7 Mamoru TASAKA 2007-01-11 16:32:12 UTC
Well, some notes:

Keep timestamps for text files as possible.
( Timestamps in http://fedoraproject.org/wiki/Packaging/Guidelines )

* Use "cp -p" or "install -p" instead of just "cp" or "install"
* for sed usage:
----------------------------------------------------
find . -type f| xargs sed -i 's/\r//' 
----------------------------------------------------
  Well, this usage of find -> sed change timestamps of all files
  under the directory ".", even some (many) files are actually
  not necessary to be fixed.

  Just use "sed" to the files which are _actually_ needed to
  be changed.

Comment 8 Michał Bentkowski 2007-01-11 16:35:40 UTC
(In reply to comment #6)
> Are you saying the geshi doc should be in /usr/share/doc/geshi or
> /usr/share/doc/pastebin-0.50/geshi?
> 

I meant the second one.

Comment 9 Dennis Gilmore 2007-01-11 16:49:06 UTC
with the find and sed removed i get 
[dennis@daedalus SPECS]$ 
rpmlint /home/dennis/fedora/RPMS/noarch/pastebin-0.50-2.noarch.rpm |grep 
wrong-script-end-of-line-encoding |wc -l
84
[dennis@daedalus SPECS]$ 
rpm -qlp /home/dennis/fedora/RPMS/noarch/pastebin-0.50-2.noarch.rpm |wc -l
106
[dennis@daedalus SPECS]$

so nearly every single file is dos line ended  in this instance i think it is 
fine to mass change line endings.  if it was a small handfull of files i would 
do them individually. but that is not the case.

as far as the docs  yeah I missed that  they should go into %doc

Comment 10 Dennis Gilmore 2007-01-11 16:53:23 UTC
as far as rpmlint goes 
[dennis@daedalus SPECS]$ 
rpmlint /home/dennis/fedora/RPMS/noarch/pastebin-0.50-2.noarch.rpm
[dennis@daedalus SPECS]$ 
rpmlint /home/dennis/fedora/RPMS/noarch/pastebin-0.50-1.noarch.rpm
[dennis@daedalus SPECS]$ 

it is indeed silent for me if there is mixed spaces/tabs  then thats should be 
fixed 

Comment 11 Michał Bentkowski 2007-01-11 17:12:41 UTC
(In reply to comment #10)
> it is indeed silent for me if there is mixed spaces/tabs  then thats should 
be 
> fixed 

Apart from built rpm file, you ought to also check rpmlint against source rpm
file.
[ecik@ecik ~]$ rpmlint ~/rpmbuild/SRPMS/pastebin-0.50-2.src.rpm
W: pastebin mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 12)


Comment 12 Mamoru TASAKA 2007-01-11 17:15:15 UTC
(In reply to comment #9)
> with the find and sed removed i get 
> [dennis@daedalus SPECS]$ 
> rpmlint /home/dennis/fedora/RPMS/noarch/pastebin-0.50-2.noarch.rpm |grep 
> wrong-script-end-of-line-encoding |wc -l
> 84
> [dennis@daedalus SPECS]$ 
> rpm -qlp /home/dennis/fedora/RPMS/noarch/pastebin-0.50-2.noarch.rpm |wc -l
> 106
> [dennis@daedalus SPECS]$
> 
> so nearly every single file is dos line ended  in this instance i think it is 
> fine to mass change line endings.  if it was a small handfull of files i would 
> do them individually. but that is not the case.

Well, even this case, still checking if the file 
"sed" command is to be applied is really a text file.
As far as I checked, there is one file which is not a text file
(./public_html/favicon.ico),
against this file "sed" command should not be used.

Comment 13 Michał Bentkowski 2007-01-11 17:40:37 UTC
(In reply to comment #12)
> Well, even this case, still checking if the file 
> "sed" command is to be applied is really a text file.
> As far as I checked, there is one file which is not a text file
> (./public_html/favicon.ico),
> against this file "sed" command should not be used.

It may be easily fixed by sedding only the files in lib/ subdirectory.

Comment 14 Michał Bentkowski 2007-01-11 18:15:37 UTC
I forgot to add that the files in public_html/ directory should be listed
explicitly.

Comment 15 Michael Stahnke 2007-01-11 19:15:53 UTC
Can you tell me why (source) I should list public_html files explicitly?  I
didn't see anything on http://fedoraproject.org/wiki/Packaging/Guidelines about it. 

I am not saying I won't do it, I just want to know why. 



Comment 16 Michał Bentkowski 2007-01-11 19:26:36 UTC
(In reply to comment #15)
> Can you tell me why (source) I should list public_html files explicitly?
> 

I meant that you ought to sed all files in lib/ and explicit list files to sed
in public_html.

Comment 17 Michael Stahnke 2007-01-11 20:05:01 UTC
Would this be an acceptable way to sed only text files? 


for file in `find . -type f`
do
   if (file $file | awk -F: '{print $2}' | grep -i text &> /dev/null) ; then
      sed -i 's/\r//g' $file
   fi
done


Comment 19 Mamoru TASAKA 2007-01-12 17:23:39 UTC
(In reply to comment #17)
> Would this be an acceptable way to sed only text files? 

This script should work well.
I can accept 0.50-3 for sed issue (not checked if other issues
exist, however I assume it is okay for now).


Note You need to log in before you can comment on or make changes to this bug.