Re: [Spacewalk-devel] Code Review Tool

[Miroslav Suchý]

Jesus M. Rodriguez wrote:

at the commit logs as mike & I do. I look at them for a few reasons:

1) understand what's going on
2) looking for glaring errors and duplicate code

While a tool might help us get better, if everyone spent sometime
reviewing the commit logs.


Agree. Good slides explaining why (and how) to do code review
http://www.aleax.it/osc08_crev.pdf


I don't think the process will work if it is up to a developer to ask
for reviews, having done that
in the past it never really worked for me.


+1
I more think about marking what has been reviewed and what not. I'm 
targeting to try avoid situation when some commits will be reviewed by 
10 people and some not looked at at all.


--
Miroslav Suchy
RHN Satellite Engineering, Red Hat

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Re: Merge branch 'master' of ssh://jlsherr...@git.fedorahosted.org/git/spacewalk

[Miroslav Suchý]

I agree with commit hooks. Anyway format bugzilla numbers is more complicated
(multiple bz numbers or none).


Wait. Why anyone would want to put two bugzilla into one commit? Either 
two BZs describe one issue - then mark one as DUPLICATE or (if one BZ is 
satellite and one spacewalk) say in satellite BZ that it is fixed in 
that Spacewalk BZ.
Or if the two BZs describe different issue you should split the commit 
into two independent commits.


--
Miroslav Suchy
RHN Satellite Engineering, Red Hat

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Fixed a iprange issue where the iprange delete URLs weren't being correctly ...

[Partha Aji]

You'd get it if you saw the iprange.jspf..
The delete link wants the complete URL, , the 
other line says   which doesnot want the /rhn ..


Partha


jmrodri wrote:

Why do we have to pass in the /rhn/ to the url here where we don't on
the others?

jesus

Sent to you by jmrodri via Google Reader: Fixed a iprange issue where
the iprange delete URLs weren't being correctly rendered via Fedora
Hosted Git Repositories - spacewalk.git/rss log by Partha Aji
 on 3/12/09 Fixed a iprange issue where the iprange
delete URLs weren't being correctly rendered
- [DH] java/code/webapp/WEB-INF/pages/kickstart/kickstartips.jsp
- [DH]
java/code/webapp/WEB-INF/pages/kickstart/wizard/profile/advanced/iprange.jsp 


Things you can do from here:
- Subscribe to Fedora Hosted Git Repositories - spacewalk.git/rss log
using Google Reader
- Get started using Google Reader to easily keep up with all your
favorite sites




___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel


___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] [PATCH] fix bootstrap scripts to retrieve server cert using SSL

[Joshua Roys]

Hello all,

My sense of paranoia tells me this should be https.

Thanks,

Joshua Roys
>From 727fb359e77d1fd5560edbe6eac552825c895167 Mon Sep 17 00:00:00 2001
From: Joshua Roys 
Date: Fri, 13 Mar 2009 08:47:56 -0400
Subject: [PATCH] fix bootstrap scripts to retrieve server cert using SSL

---
 spacewalk/certs-tools/rhn_bootstrap_strings.py |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/spacewalk/certs-tools/rhn_bootstrap_strings.py b/spacewalk/certs-tools/rhn_bootstrap_strings.py
index d991d4e..1705844 100644
--- a/spacewalk/certs-tools/rhn_bootstrap_strings.py
+++ b/spacewalk/certs-tools/rhn_bootstrap_strings.py
@@ -248,10 +248,10 @@ echo
 echo "* attempting to install corporate public CA cert"
 if [ $USING_SSL -eq 1 ] ; then
 if [ $ORG_CA_CERT_IS_RPM_YN -eq 1 ] ; then
-rpm -Uvh ${HTTP_PUB_DIRECTORY}/${ORG_CA_CERT}
+rpm -Uvh ${HTTPS_PUB_DIRECTORY}/${ORG_CA_CERT}
 else
 rm -f ${ORG_CA_CERT}
-$FETCH ${HTTP_PUB_DIRECTORY}/${ORG_CA_CERT}
+$FETCH ${HTTPS_PUB_DIRECTORY}/${ORG_CA_CERT}
 mv ${ORG_CA_CERT} /usr/share/rhn/
 fi
 fi
-- 
1.6.2

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] fix bootstrap scripts to retrieve server cert using SSL

[Jan Pazdziora]

On Fri, Mar 13, 2009 at 09:33:36AM -0400, Joshua Roys wrote:
> Hello all,
>
> My sense of paranoia tells me this should be https.

Will the https work, without certificates available on the client?

-- 
Jan Pazdziora
Senior Software Engineer, Satellite Engineering, Red Hat

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Code Review Tool

[James Bowes]

On Thu, Mar 12, 2009 at 03:31:53PM -0400, Jesus M. Rodriguez wrote:
> 2009/3/12 Michael Mraka :
> > Pradeep Kilambi wrote:
> > % Miroslav Suchý wrote:
> > % >Seeing as Jesus likes reviewing commit... I'm just wondering - do we
> > % >want to setup some Code Review tools? Will we use it? Or gitk/tig and
> > % >sending mails to spacewalk-devel is sufficient?
> > % >If I see here majority of positive response I will be happy to set up
> > % >some tool. Hmm probably some time after space05 when things will slows
> > % >down.
> > %
> > %
> > % I like the idea, sounds nice. But how do we plan on using it? Unless
> > % people volunteer to post their commits to the tool and request for a
> > % review, I think we'll be in the same position again where zeus or mike
> > % or someone else looking at the commits feeds and suggesting changes.
> > % I think if we have a tool like such, it comes down to the discipline of
> > % individual developer  to  ask for a review.  Honestly I dont see that
> > % happening very frequently :)
> >
> > I like the idea either. IMHO "post" commit review would be enough (we do it
> > now when reading commit logs) so commits could be taken automaticaly from 
> > git
> > and then "wait" for review by another developer.
> 
> I would be game for having a tool. I'd also be happy if others would simply 
> look
> at the commit logs as mike & I do. I look at them for a few reasons:
> 
> 1) understand what's going on
> 2) looking for glaring errors and duplicate code
> 
> While a tool might help us get better, if everyone spent sometime
> reviewing the commit logs.
> I don't think the process will work if it is up to a developer to ask
> for reviews, having done that
> in the past it never really worked for me.


Why not take away everyone's commit access and make all changes go
through the mailing list first? I've never liked standalone code review
tools, because they make me leave my safe cave-like environment of vim,
mutt, and the command line. Patches sent to mailing lists for review
before commit let me use the tools I already know and love; reading
commit logs do the same thing, but at that point it's too late, in a
sense.

> 
> jesus
> 
> ___
> Spacewalk-devel mailing list
> Spacewalk-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/spacewalk-devel

-James


pgp10187g3Bvl.pgp
Description: PGP signature
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] [PATCH] fix bootstrap scripts to retrieve server cert using SSL

[Joshua Roys]

Jan Pazdziora wrote:

On Fri, Mar 13, 2009 at 09:33:36AM -0400, Joshua Roys wrote:

Hello all,

My sense of paranoia tells me this should be https.


Will the https work, without certificates available on the client?



If wget is used with the --no-check-certificate option, which is checked 
for availability in the bootstrap.sh script.  curl uses -k, which does 
the same thing.  But I guess it's a chicken and egg thing - maybe we 
could print out some of the certificate info and hope the admin checks 
it by hand?


___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

[Spacewalk-devel] PGPORT: additional packages

[Vikram Rai]

Hi Devan,

Attached are additional packages that need to be checked into the
repository.
Since I do not have rights to check them in, could you please check them
into the following location:

spacewalk/schema/spacewalk/postgresql/packages/

Thanks

Vikram Rai
EnterpriseDB


rhn_packages.tar.gz
Description: GNU Zip compressed data
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Code Review Tool

[Jesus M. Rodriguez]

On Fri, Mar 13, 2009 at 9:45 AM, James Bowes  wrote:
> Why not take away everyone's commit access and make all changes go
> through the mailing list first? I've never liked standalone code review
> tools, because they make me leave my safe cave-like environment of vim,
> mutt, and the command line. Patches sent to mailing lists for review
> before commit let me use the tools I already know and love; reading
> commit logs do the same thing, but at that point it's too late, in a
> sense.

Because we would implode with the sheer number of commits.  I think reviewing
every commit before hand would cause such a bottleneck that it would become
unsustainable.

I guess my attempt at having folks participate in reviewing commits is to help
each team member learn from each other. For example, if I wrote a bunch of pages
that use ListTag and you are knew to it, I'd expect you to do
something completely
different and I can educate you on what we've already done. Also, I'd
like folks look
over my stuff (which I never get enough of -- and no 'you code?' jokes) :)

jesus

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Code Review Tool

[Miroslav Suchý]

James Bowes wrote:

Why not take away everyone's commit access and make all changes go
through the mailing list first? 


E.g. Google has it this way. And it was enough PITA that I did not 
finished that process and take my patch for their project back.



Patches sent to mailing lists for review
before commit let me use the tools I already know and love; reading
commit logs do the same thing, but at that point it's too late, in a
sense.


Do we want to have nice and clean repo, without accidental or partially 
wrong commits? I think we do not need it. It will slows downs the work.
It is better to commit earlier and if you done it wrong, then correct or 
revert later.


--
Miroslav Suchy
RHN Satellite Engineering, Red Hat

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Code Review Tool

[Jesus M. Rodriguez]

On Fri, Mar 13, 2009 at 9:57 AM, Miroslav Suchý  wrote:
> James Bowes wrote:
>>
>> Why not take away everyone's commit access and make all changes go
>> through the mailing list first?
>
> E.g. Google has it this way. And it was enough PITA that I did not finished
> that process and take my patch for their project back.
>
>> Patches sent to mailing lists for review
>> before commit let me use the tools I already know and love; reading
>> commit logs do the same thing, but at that point it's too late, in a
>> sense.
>
> Do we want to have nice and clean repo, without accidental or partially
> wrong commits? I think we do not need it. It will slows downs the work.
> It is better to commit earlier and if you done it wrong, then correct or
> revert later.

+1 to all of your comments Miroslav. /me thinks I broke a rule here by not
adding anything meaningful to the thread :)

jesus

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] PGPORT: additional packages

[Jesus M. Rodriguez]

On Fri, Mar 13, 2009 at 9:48 AM, Vikram Rai  wrote:
> Hi Devan,
>
> Attached are additional packages that need to be checked into the
> repository.
> Since I do not have rights to check them in, could you please check them
> into the following location:
>
> spacewalk/schema/spacewalk/postgresql/packages/
>
> Thanks
>
> Vikram Rai
> EnterpriseDB
>
> ___
> Spacewalk-devel mailing list
> Spacewalk-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/spacewalk-devel
>


Vikram,

Thanks for this submission, Devan and/or Jeff will look it over.

Thanks
jesus

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] PGPORT: additional packages

[Devan Goodwin]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On Fri, 13 Mar 2009 19:18:22 +0530
Vikram Rai  wrote:

> Hi Devan,
> 
> Attached are additional packages that need to be checked into the
> repository.
> Since I do not have rights to check them in, could you please check
> them into the following location:
> 
> spacewalk/schema/spacewalk/postgresql/packages/
> 
> Thanks
> 
> Vikram Rai
> EnterpriseDB

Can do, thanks Vikram!

Devan

- -- 
  Devan Goodwin 
  Software Engineer Spacewalk / RHN Satellite
  Halifax, Canada   650.567.9039x79267
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.9 (GNU/Linux)

iEYEARECAAYFAkm6aVsACgkQAyHWaPV9my77wQCg430oYfU5T0asinFdogKCKEWC
AFEAnis4S7FFEfI9Km/eJ3szeZ+F1kav
=svX1
-END PGP SIGNATURE-

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Code Review Tool

[James Bowes]

On Fri, Mar 13, 2009 at 10:05:58AM -0400, Jesus M. Rodriguez wrote:
> On Fri, Mar 13, 2009 at 9:57 AM, Miroslav Suchý  wrote:
> > James Bowes wrote:
> >>
> >> Why not take away everyone's commit access and make all changes go
> >> through the mailing list first?
> >
> > E.g. Google has it this way. And it was enough PITA that I did not finished
> > that process and take my patch for their project back.

Well nobody's going to give a first time contributor commit access right
away, are they? In my own experiences, going through a review of a patch
on a mailing list has been quite usefull. And isn't this how occasional
contributors to spacewalk already submit changes?

> >
> >> Patches sent to mailing lists for review
> >> before commit let me use the tools I already know and love; reading
> >> commit logs do the same thing, but at that point it's too late, in a
> >> sense.
> >
> > Do we want to have nice and clean repo, without accidental or partially
> > wrong commits? I think we do not need it. It will slows downs the work.
> > It is better to commit earlier and if you done it wrong, then correct or
> > revert later.

That's actually not what I meant. I mean that having people to opt-in to
review other's code by looking at a commit list is obviously not being
very effective in spacewalk's case, or else you wouldn't be looking at
other review methods. In hindsight, I didn't express this well at all.
So my first email fails review.

> 
> +1 to all of your comments Miroslav. /me thinks I broke a rule here by not
> adding anything meaningful to the thread :)
> 

Nah, you're the guy doing most of the existing review, so you get a pass
here.

Anyhoo, I'm much less vested in this than you two are, so you probably
know what's more useful for you team.

> jesus
> 
> ___
> Spacewalk-devel mailing list
> Spacewalk-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/spacewalk-devel

-James


pgpVRgsjXJ4HW.pgp
Description: PGP signature
___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] Code Review Tool

[Miroslav Suchý]

James Bowes wrote:

Well nobody's going to give a first time contributor commit access right
away, are they? In my own experiences, going through a review of a patch
No, I attached patch to BZ and expected that somebody will pick it up. 
But they forced me to go through review tool (including creating yet 
another account) and sign off some documents ...



--
Miroslav Suchy
RHN Satellite Engineering, Red Hat

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] 489792- fixing incorrect api return types

[Justin Sherrill]

jmrodri wrote:
> Instead of
> 
> + List list = (List) listErrata(sessionKey, channelLabel, "", "");
> + //
> + for (Map item : list) {
> + item.put("date", item.get("issue_date"));
> + }
> + return list;
> 
> why not change the query to return date as the column name instead of
> issue_date OR
> change it to return both issue_date and date together. That avoids having to
> loop through the list simply to change the keys.


Because 'date' is a reserved word evidently.  It just wouldn't work.

> 
> Also, remove the empty // line.
> 
> jesus
> 
>  
>  
> 
> 
>   Sent to you by jmrodri via Google Reader:
> 
>  
>  
> 
> 
> 489792- fixing incorrect api return types
> 
> 
> 
> via Fedora Hosted Git Repositories - spacewalk.git/rss log
>  by Justin
> Sherrill  on 3/12/09
> 
> 489792- fixing incorrect api return types
> 
> * [D
>   
> H
>   
> ]
>   java/code/src/com/redhat/rhn/common/db/datasource/xml/Errata_queries.xml
> * [D
>   
> H
>   
> ]
>   
> java/code/src/com/redhat/rhn/frontend/xmlrpc/channel/software/ChannelSoftwareHandler.java
> * [D
>   
> H
>   
> ]
>   
> java/code/src/com/redhat/rhn/frontend/xmlrpc/channel/software/test/ChannelSoftwareHandlerTest.java
> 
> 
>  
>  
> 
> 
>   Things you can do from here:
> 
> * Subscribe to Fedora Hosted Git Repositories - spacewalk.git/rss
>   log
>   
> 
>   using *Google Reader*
> * Get started using Google Reader
>    to easily keep up
>   with *all your favorite sites*
> 
>  
>  
> 
> 
> 
> 
> ___
> Spacewalk-devel mailing list
> Spacewalk-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/spacewalk-devel


-- 
Justin Sherrill, RHCA  1801 Varisty Drive.
Software EngineerRaleigh, NC 27603
Red Hat, Inc.

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] 489775 - fixing listERrata api due to bad query

[Justin Sherrill]

jmrodri wrote:
> why is this a BAD query? I think this was done on purpose to return the
> issue_date as date.
> 
> jesus
> 
>  

The query was 'bad' because evidently date is a reserved word.   Try to
do anything as  'select id as date from rhnServer'.  Wont' work.  Feel
free to look at the referenced bug for the sql error.

-Justin



>  
> 
> 
>   Sent to you by jmrodri via Google Reader:
> 
>  
>  
> 
> 
> 489775 - fixing listERrata api due to bad query
> 
> 
> 
> via Fedora Hosted Git Repositories - spacewalk.git/rss log
>  by Justin
> Sherrill  on 3/12/09
> 
> 489775 - fixing listERrata api due to bad query
> 
> * [D
>   
> H
>   
> ]
>   java/code/src/com/redhat/rhn/common/db/datasource/xml/Errata_queries.xml
> 
> 
>  
>  
> 
> 
>   Things you can do from here:
> 
> * Subscribe to Fedora Hosted Git Repositories - spacewalk.git/rss
>   log
>   
> 
>   using *Google Reader*
> * Get started using Google Reader
>    to easily keep up
>   with *all your favorite sites*
> 
>  
>  
> 
> 
> 
> 
> ___
> Spacewalk-devel mailing list
> Spacewalk-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/spacewalk-devel


-- 
Justin Sherrill, RHCA  1801 Varisty Drive.
Software EngineerRaleigh, NC 27603
Red Hat, Inc.

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] 489775 - fixing listERrata api due to bad query

[Jan Pazdziora]

On Fri, Mar 13, 2009 at 10:55:18AM -0400, Justin Sherrill wrote:
> jmrodri wrote:
> > why is this a BAD query? I think this was done on purpose to return the
> > issue_date as date.
> 
> The query was 'bad' because evidently date is a reserved word.   Try to
> do anything as  'select id as date from rhnServer'.  Wont' work.  Feel
> free to look at the referenced bug for the sql error.

You are looking for

select id as "DATE" from rhnServer

here, probably.

-- 
Jan Pazdziora
Senior Software Engineer, Satellite Engineering, Red Hat

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel

Re: [Spacewalk-devel] 489775 - fixing listERrata api due to bad query

[Justin Sherrill]

Jan Pazdziora wrote:
> On Fri, Mar 13, 2009 at 10:55:18AM -0400, Justin Sherrill wrote:
>> jmrodri wrote:
>>> why is this a BAD query? I think this was done on purpose to return the
>>> issue_date as date.
>> The query was 'bad' because evidently date is a reserved word.   Try to
>> do anything as  'select id as date from rhnServer'.  Wont' work.  Feel
>> free to look at the referenced bug for the sql error.
> 
> You are looking for
> 
>   select id as "DATE" from rhnServer
> 
> here, probably.
> 
yeah, yeah, yea  :}

-- 
Justin Sherrill, RHCA  1801 Varisty Drive.
Software EngineerRaleigh, NC 27603
Red Hat, Inc.

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel