(In reply to pphan from comment #0)
> Spec URL: https://www.dropbox.com/s/a3pvkh4iv8r7zd5/weatherman.spec

Can you host your spec at http://matrix.senecac.on.ca/~pphan/weatherman/ also?

We don't like dropbox.

> SRPM URL:
> http://matrix.senecac.on.ca/~pphan/weatherman/weatherman-1.2.2-1.fc18.src.rpm
> Description: 
> weatherman displays weather information from weatherbug.com on the command
> line. It is written entirely in bash and will run on most UNIX/Linux systems.
> 
> The latest version of weatherman was released on January 12, 2013 with the
> following changes:
> 
> Using metric units with extended forecasts works once again
> Better column alignment when metric units are specified
> Fixed a rare case where extended forecasts would be displayed incorrectly
> No longer display units with empty values
> Removed the program name and project URL from the output

We don't need changelog as description.

> Fedora Account System Username: pphan2

We can't find your account so far.

(In reply to Christopher Meng from comment #1)
> (In reply to pphan from comment #0)
> We don't like dropbox.

Who is we? Submitters are free to choose whatever hosting service is suiteable to them.

(In reply to Ralf Corsepius from comment #2)
> (In reply to Christopher Meng from comment #1)
> > (In reply to pphan from comment #0)
> > We don't like dropbox.
> 
> Who is we? Submitters are free to choose whatever hosting service is
> suiteable to them.

Yes, but that's troublesome for reviewers, fedora-review -b won't work.

Sorry, 
Not sure how I edit my post above so Ill reply to you. 
I upload to my school storage.
Spec URL: http://matrix.senecac.on.ca/~pphan/weatherman/weatherman.spec
SRPM URL: http://matrix.senecac.on.ca/~pphan/weatherman/weatherman-1.2.2-1.fc18.src.rpm

CORRECT Fedora Account System Username: pphan12 

(In reply to Christopher Meng from comment #1)
> (In reply to pphan from comment #0)
> > Spec URL: https://www.dropbox.com/s/a3pvkh4iv8r7zd5/weatherman.spec
> 
> Can you host your spec at http://matrix.senecac.on.ca/~pphan/weatherman/
> also?
> 
> We don't like dropbox.
> 
> > SRPM URL:
> > http://matrix.senecac.on.ca/~pphan/weatherman/weatherman-1.2.2-1.fc18.src.rpm
> > Description: 
> > weatherman displays weather information from weatherbug.com on the command
> > line. It is written entirely in bash and will run on most UNIX/Linux systems.
> > 
> > The latest version of weatherman was released on January 12, 2013 with the
> > following changes:
> > 
> > Using metric units with extended forecasts works once again
> > Better column alignment when metric units are specified
> > Fixed a rare case where extended forecasts would be displayed incorrectly
> > No longer display units with empty values
> > Removed the program name and project URL from the output
> 
> We don't need changelog as description.
> 
> > Fedora Account System Username: pphan2
> 
> We can't find your account so far.

Hi pphan12,

I am also a new packager and here are some quick comments on the package. If something I mention here is wrong, I am sure someone will correct it.

* source0: url is pasted twice.

* License: BSD - The word 'license' need not be mentioned. It will cause rpmlint to throw errors.

* %description - This section should have lines less than 80 characters. You can take a look at the common rpmlint errors wiki page.

* Requires: curl - It is my belief that curl need not be added as requires. As far as I know it is not possible to remove curl from the system.

* %post and % preun scriptlets - the source file does not seem provide any info manual. So, I am not sure these scriptlets are needed.

* changelog - please check the format with other packages in the repository. Small changes are needed.

Hope I have not made any mistakes here .. :) Good luck.

Hello,

As a candidate packager for Fedora, and if you still want to produce this package, I could make an unofficial review for your proposed package.

I agree with all the point Mukudan gave you.

As the shell script of this package is made for bash (specific use of bash arrays), it would probably better to explicitly indicate that it is required in the requires section of the spec file, even if bash and curl are "low-level" needed packages for the system.

As the upstream web site points to github you should define global variables at the beginning of your spec file:

%global commit 0fe94cb2fb10ef556e82dcce22260717c579f87a
%global shortcommit %(c=%{commit}; echo ${c:0:7})

and then use them as this:

Source0:        https://github.com/subrosa2/weatherman/archive/%{commit}/weatherman-%{commit}.tar.gz

or like this:

%setup -qn %{name}-%{commit}

in order to pass rpmlint tests.

You should also use the directories predefined macros like this:

%install
install -D -m 0755 weatherman %{buildroot}%{_bindir}/weatherman
install -D -m 0644 weatherman.1.gz  %{buildroot}%{_mandir}/man1/weatherman.1.gz

%files
%{_bindir}/weatherman
%doc CHANGES LICENSE
%{_mandir}/man1/%{name}.1*

Maybe it would be also useful to had an empty %build section to avoid rpmlint warning.

If needed, I could provide you a modified spec file as an example.

Let me know if you want me to make an unofficial review of your package, once you modified your spec file.

Cordially,


-- 
NVieville

Hello,

I wonder if you could drop a line if you still interested in this review request and my proposed unofficial review. 

Thanks in advance.

Cordially,


-- 
NVieville

Closing due long inactivity. Feel free to reopen if you want to continue.