Re: Package Review Request: python-picloud

2012-03-06 Thread Amit Saha 

On 03/06/2012 09:07 PM, Paul Howarth wrote:

On 03/06/2012 03:11 AM, Vijay N. Majagaonkar wrote:


Please, no! %{__cp} hugely decreases readability and if the
situation happens that mkdir and cp are not in the $PATH we will
have much bigger problems than running sed on all .spec files.


I am sorry but this will hit even if you don't use macro when tools are
not in $PATH, unless you use full path

Use just plain Unix commands as $DEITY intended them to be used.

I believe macro give you plain Unix command with full path.


It does, but the current packaging guidelines say not to use these macros:

http://fedoraproject.org/wiki/Packaging:Guidelines#Macros

"Macro forms of system executables SHOULD NOT be used except
when there is a need to allow the location of those executables
to be configurable. For example, rm should be used in preference
to %{__rm}, but %{__python} is acceptable."

When reviewing a package, you need to be familiar with the guidelines
and not suggest changes that are in contradiction with them.


Thanks for the clarifications and comments. Will it sound like begging 
if I call out for someone to review this, please?


Thanks much!
-Amit

--
http://echorand.me
--
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel

Re: Package Review Request: python-picloud

2012-03-06 Thread Paul Howarth 

On 03/06/2012 03:11 AM, Vijay N. Majagaonkar wrote:


Please, no! %{__cp} hugely decreases readability and if the
situation happens that mkdir and cp are not in the $PATH we will
have much bigger problems than running sed on all .spec files.


I am sorry but this will hit even if you don't use macro when tools are
not in $PATH, unless you use full path

Use just plain Unix commands as $DEITY intended them to be used.

I believe macro give you plain Unix command with full path.


It does, but the current packaging guidelines say not to use these macros:

http://fedoraproject.org/wiki/Packaging:Guidelines#Macros

  "Macro forms of system executables SHOULD NOT be used except
   when there is a need to allow the location of those executables
   to be configurable. For example, rm should be used in preference
   to %{__rm}, but %{__python} is acceptable."

When reviewing a package, you need to be familiar with the guidelines 
and not suggest changes that are in contradiction with them.


Paul.
--
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel

Re: Package Review Request: python-picloud

2012-03-05 Thread Vijay N. Majagaonkar 
> Please, no! %{__cp} hugely decreases readability and if the situation
> happens that mkdir and cp are not in the $PATH we will have much bigger
> problems than running sed on all .spec files.


I am sorry but this will hit even if you don't use macro when tools are not
in $PATH, unless you use full path


> Use just plain Unix commands as $DEITY intended them to be used.
>
I believe macro give you plain Unix command with full path.



;)
V!jay
-- 
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel

Re: Package Review Request: python-picloud

2012-03-05 Thread Matej Cepl 

>> mkdir -p 
>>cp -p ...
It will be good if you make use of macro like %{__cp}

Please, no! %{__cp} hugely decreases readability and if the situation 
happens that mkdir and cp are not in the $PATH we will have much bigger 
problems than running sed on all .spec files. Use just plain Unix 
commands as $DEITY intended them to be used.


Matěj
--
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel

Re: Package Review Request: python-picloud

2012-03-05 Thread Vijay N. Majagaonkar 
I am not reviewer but I think it make sense following things are good
to incorporate

>># These packages are not require for python >=2.6 but required for python
= 2.5

>>Requires:   python >= 2.5

I think your Requires: must be python = 2.5

>>%setup -q -n cloud-%version

is that version should be %{version}

>> mkdir -p ...

>>cp -p ...

It will be good if you make use of macro like %{__cp}



;)

V!jay

On Mon, Mar 5, 2012 at 1:00 AM, Amit Saha  wrote:

> Hello:
>
> Could someone please review my first package: https://bugzilla.redhat.com/
> **show_bug.cgi?id=799810?
>
> Since, this is my first, I am in need of a sponsor (FAS: amitksaha)
>
> Thanks a lot !
> -Amit
> --
> http://echorand.me
> --
> devel mailing list
> devel@lists.fedoraproject.org
> https://admin.fedoraproject.**org/mailman/listinfo/devel
-- 
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel