Re: [Spacewalk-devel] On quality of patches

2014-03-05 Thread Lamont Peterson 
On 3/5/14, 2:58 AM, "Johannes Renner"  wrote:

>On 03/05/2014 08:04 AM, Lamont Peterson wrote:
>> On 3/4/14, 12:31 AM, "Silvio Moioli"  wrote:
>> 
 From: Paul Robert Marino >>> >
 I think we need a map of what these shared file and functions effect
so
 we can do more formal QA testing in the future
>>>
>>> On 03/03/2014 08:09 PM, Lamont Peterson wrote:
 Agreed.
>>>
>>> Unfortunately I think such a "map", if ever existed, would be far too
>>> complex to be of any practical value. Consider this image, which "maps"
>>> only database tables, a relatively small fraction of the total software
>>> complexity:
>>>
>>> http://turing.suse.de/~smoioli/relationships.real.compact.png
>>>
>>> Developers already have tools in modern IDEs such as Eclipse to check
>>> for (partial) call trees[1], and are expected to use them. I do not
>>> think we can do much more than that in this regard, of course new
>>> opinions are welcome :-)
>>>
>>> Cheers,
>>>
>>> [1] http://eclipse-tools.sourceforge.net/call-hierarchy/usage.html
>>> -- 
>>> Silvio Moioli
>>> SUSE LINUX Products GmbH
>>> Maxfeldstraße 5, 90409 Nürnberg Germany
>>>
>>> ___
>>> Spacewalk-devel mailing list
>>> Spacewalk-devel@redhat.com
>>> https://www.redhat.com/mailman/listinfo/spacewalk-devel
>>>
>> 
>> You’re right (and thanks for the vivid DB relationships diagram).  In my
>> mind, the auto-generated map should simply be a calling-me list:
>> 
>> Function A is called by:
>>   file.java   : 45
>>   file2.java  : 134
>> 
>> That’s for developers.  The map for QA could be simpler and generated
>>from
>> merely stuff one can hit “submit” on?
>
>Hey,
>
>I agree with Silvio: as a developer I don't need such a map. It would be
>much
>harder to lookup something on this map instead of just using the features
>of
>my IDE that were made for that purpose (like e.g. "References",
>"Declarations"
>or "Type Hierarchy" in Eclipse).
>
>Further note that there are relations in the codebase that are hard to
>figure
>out using automatic tooling. I therefore personally prefer to use the
>very good
>search features in IDEs to maintaining some automatic tools that need to
>have a
>lot of knowledge in order to find relationships. Think of e.g. translation
>strings, jsp files, struts-config.xml, form validation *.xsd files,
>*.hbm.xml
>files or dwr.xml.
>
>Regards,
>Johannes

Hi, Johannes,

I agree with you both on this; as a developer, the IDE tools are a great
way to accomplish this.  Especially for those already familiar with some
portion of the architecture.

However, for the QA folks, something simpler, without them resorting to
grok’ing code, could be quite useful. We have lots of automated testing,
and that’s good, but a QA group needs good info to base regression testing
on.  Wouldn’t that part of this (odd) thread be useful as we strive to
improve future process for projects such as this?

Hey, I’m not a known contributor around here (yet); I’m just spitballing.
-- 
Lamont Peterson
Sr. Systems Administrator | Unix Systems Operations
Intermountain Healthcare
Office: 801.442.6497 | lamont.peter...@imail.org


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

Re: [Spacewalk-devel] On quality of patches

2014-03-05 Thread Johannes Renner 
On 03/05/2014 08:04 AM, Lamont Peterson wrote:
> On 3/4/14, 12:31 AM, "Silvio Moioli"  wrote:
> 
>>> From: Paul Robert Marino >> >
>>> I think we need a map of what these shared file and functions effect so
>>> we can do more formal QA testing in the future
>>
>> On 03/03/2014 08:09 PM, Lamont Peterson wrote:
>>> Agreed.
>>
>> Unfortunately I think such a "map", if ever existed, would be far too
>> complex to be of any practical value. Consider this image, which "maps"
>> only database tables, a relatively small fraction of the total software
>> complexity:
>>
>> http://turing.suse.de/~smoioli/relationships.real.compact.png
>>
>> Developers already have tools in modern IDEs such as Eclipse to check
>> for (partial) call trees[1], and are expected to use them. I do not
>> think we can do much more than that in this regard, of course new
>> opinions are welcome :-)
>>
>> Cheers,
>>
>> [1] http://eclipse-tools.sourceforge.net/call-hierarchy/usage.html
>> -- 
>> Silvio Moioli
>> SUSE LINUX Products GmbH
>> Maxfeldstraße 5, 90409 Nürnberg Germany
>>
>> ___
>> Spacewalk-devel mailing list
>> Spacewalk-devel@redhat.com
>> https://www.redhat.com/mailman/listinfo/spacewalk-devel
>>
> 
> You’re right (and thanks for the vivid DB relationships diagram).  In my
> mind, the auto-generated map should simply be a calling-me list:
> 
> Function A is called by:
>   file.java   : 45
>   file2.java  : 134
> 
> That’s for developers.  The map for QA could be simpler and generated from
> merely stuff one can hit “submit” on?

Hey,

I agree with Silvio: as a developer I don't need such a map. It would be much
harder to lookup something on this map instead of just using the features of
my IDE that were made for that purpose (like e.g. "References", "Declarations"
or "Type Hierarchy" in Eclipse).

Further note that there are relations in the codebase that are hard to figure
out using automatic tooling. I therefore personally prefer to use the very good
search features in IDEs to maintaining some automatic tools that need to have a
lot of knowledge in order to find relationships. Think of e.g. translation
strings, jsp files, struts-config.xml, form validation *.xsd files, *.hbm.xml
files or dwr.xml.

Regards,
Johannes

-- 
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

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

Re: [Spacewalk-devel] On quality of patches

2014-03-04 Thread Lamont Peterson 
On 3/4/14, 12:31 AM, "Silvio Moioli"  wrote:

>> From: Paul Robert Marino >>
>> I think we need a map of what these shared file and functions effect so
>> we can do more formal QA testing in the future
>
>On 03/03/2014 08:09 PM, Lamont Peterson wrote:
>> Agreed.
>
>Unfortunately I think such a "map", if ever existed, would be far too
>complex to be of any practical value. Consider this image, which "maps"
>only database tables, a relatively small fraction of the total software
>complexity:
>
>http://turing.suse.de/~smoioli/relationships.real.compact.png
>
>Developers already have tools in modern IDEs such as Eclipse to check
>for (partial) call trees[1], and are expected to use them. I do not
>think we can do much more than that in this regard, of course new
>opinions are welcome :-)
>
>Cheers,
>
>[1] http://eclipse-tools.sourceforge.net/call-hierarchy/usage.html
>-- 
>Silvio Moioli
>SUSE LINUX Products GmbH
>Maxfeldstraße 5, 90409 Nürnberg Germany
>
>___
>Spacewalk-devel mailing list
>Spacewalk-devel@redhat.com
>https://www.redhat.com/mailman/listinfo/spacewalk-devel
>

You’re right (and thanks for the vivid DB relationships diagram).  In my
mind, the auto-generated map should simply be a calling-me list:

Function A is called by:
  file.java   : 45
  file2.java  : 134

That’s for developers.  The map for QA could be simpler and generated from
merely stuff one can hit “submit” on?
-- 
Lamont Peterson
Sr. Systems Administrator | Unix Systems Operations
Intermountain Healthcare
Office: 801.442.6497 | lamont.peter...@imail.org


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

Re: [Spacewalk-devel] On quality of patches

2014-03-03 Thread Silvio Moioli 
> From: Paul Robert Marino mailto:prmari...@gmail.com>>
> I think we need a map of what these shared file and functions effect so
> we can do more formal QA testing in the future

On 03/03/2014 08:09 PM, Lamont Peterson wrote:
> Agreed.

Unfortunately I think such a "map", if ever existed, would be far too
complex to be of any practical value. Consider this image, which "maps"
only database tables, a relatively small fraction of the total software
complexity:

http://turing.suse.de/~smoioli/relationships.real.compact.png

Developers already have tools in modern IDEs such as Eclipse to check
for (partial) call trees[1], and are expected to use them. I do not
think we can do much more than that in this regard, of course new
opinions are welcome :-)

Cheers,

[1] http://eclipse-tools.sourceforge.net/call-hierarchy/usage.html
-- 
Silvio Moioli
SUSE LINUX Products GmbH
Maxfeldstraße 5, 90409 Nürnberg Germany

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

Re: [Spacewalk-devel] On quality of patches

2014-03-03 Thread Lamont Peterson 
Agreed.

Such a map needs to be generated from source, please?
--
Lamont Peterson
Sr. Systems Administrator | Unix Systems Operations

[cid:image002.jpg@01CEC98D.A33033E0]<http://intermountainhealthcare.org/>
From: Paul Robert Marino mailto:prmari...@gmail.com>>
Reply-To: "spacewalk-devel@redhat.com<mailto:spacewalk-devel@redhat.com>" 
mailto:spacewalk-devel@redhat.com>>
Date: Thursday, January 30, 2014 at 9:50 AM
To: "spacewalk-devel@redhat.com<mailto:spacewalk-devel@redhat.com>" 
mailto:spacewalk-devel@redhat.com>>
Subject: Re: [Spacewalk-devel] On quality of patches

There is an other factor here that no one has mentioned. Recently I've found 
and reported a lot of bugs in the nightly repo. Where they came from is usually 
some one updates a portion of the code to fix a specific issue but since a 
portion of that code is called by multiple parts of the web interface it may 
break other pages than the one the developer is focused on.
I think we need a map of what these shared file and functions effect so we can 
do more formal QA testing in the future



-- Sent from my HP Pre3


On Jan 30, 2014 7:33, Duncan Mac-Vicar P. 
mailto:dmacvi...@suse.de>> wrote:

On 30/01/14 11:57, Matej Kollar wrote:
> Hi all.
>
> We welcome community contributions and we want to maintain some level
> of quality. Natural expectation is that every proposed patch is
> tested for the functionality.

I fully agree with you that this is not acceptable.

Now, I think your diagnostic is IMHO neither fair or correct. This does
not have to do with the "community contributions" but with the setup of
the project itself.
We sometimes have sent patches that are very strictly reviewed,
sometimes needing change multiple times to get a reviewer happy.

Then next day our internal testsuite fails only to realize that someone
with direct commit access did a big refactoring and committed very
broken code in a big patch that was not reviewed by anyone and which
quality was also obviously not the best. We asked ourselves, what is the
point of reviewing "some of them"?. Not only code but also design
decisions and way of approaching solutions.

Sometimes this happens with our own patches, depending on the reviewer,
they may get committed faster.

The setup of the review process is broken.

- All code should be committed in similar units (features/branches)
- All code should be able to be reviewed and vetoed by everyone

OpenStack has this model working quite successfully. Every patch is
reviewed with +1, and they need a certain amounts of ACKS to get
committed. Everyone can review and people learn in the process, and it
is a great source of inspiration for other projects.

We care about quality and you can see that we not only contributed our
own testsuite in early 2011 with the first release of our own product,
but then focused lot of contributions in having the testsuites enabled
and working again.

But all this is pointless if the process has holes. Right now it depends
on a lot of luck to be useful.

We suggested not long time ago in the mailing list to move to a github
approach where code could be submitted only with pull-requests that need
to be reviewed. Policies could be created that at least 2-3 acks is
needed to merge a feature. People can study those reviews and learn in
the process.
Continuous integration could be setup on top of this process, to have up
to date results.

But what you are seeing, the process is kind of designed to produce
those results, which is a process where code has different ways to
arrive to master, which different quality outcomes.

--
Duncan Mac-Vicar P. - http://www.suse.com/

SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix
Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstraße 5, 90409 Nürnberg, Germany

___
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com<mailto: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

Re: [Spacewalk-devel] On quality of patches

2014-03-03 Thread Lamont Peterson 
On 1/30/14, 3:57 AM, "Matej Kollar"  wrote:

>Hi all.
>
>We welcome community contributions and we want to maintain some level
>of quality. Natural expectation is that every proposed patch is
>tested for the functionality.
>
>However, recently I came across quite nasty little thing in Spacewalk...
>
>Using SSM to schedule reboot one might end up very unpleasantly surprised,
>expecting it not to occur for specified period of time, when in fact it
>is being scheduled "as soon as possible", no matter what you try.
>
>(There was no way it ever worked - parameters for forms were not passed
>to script in request (where DatePicker looks for them) and new Date
>object was returned. Well, someone scamped his work, so I had to do it
>for him last night. And I consider myself lucky that I found out...
>As en exercise in applied imagination, try to imagine OSAD running on
>such machine. To stretch your imagination even further, imagine
>the machine acting as a life supporting computer during child surgery...)

Nice example :) .  FYI, around here (Intermountain Healthcare), any
activity requiring a planned reboot of a server would also require an SA
to be on “live” and monitoring the system and would likely involve a DBA
or App Admin to be online to confirm all services are properly
started/running.  Therefore, this example, while great, wouldn’t happen in
our industry because we would never use such functionality (yes, mistakes
can happen).

BTW, there are systems that can directly impact patient care in general
bed space, ICU, NICU, ED, OR and other facilities. So this example is an
excellent idea in general to keep in mind.

Yes, most systems in the world will not cause harm to life, limb or
property should something go wrong, but there are a lot of places where it
could (not just health care, but think about Air Traffic Control,
City-wide Traffic Management, National Weather Service, Automated
Industrial/Manufacturing and the list could go on and on).

In the other end of the scale, what if something went sideways in a small
company?  People with 10 servers to manage or 100 servers and 5 employees
can go out of business if their stuff is down/broken (by systems
management software, like Spacewalk?).  That’s a huge impact to their
worlds.

>Btw: next time have a peek into struts-config.xml and look for
>form-bean and form-property tags... say like in
>eea3c6320bfc3dae8532844a7c0e71dcc5712c39.
>
>To continue with our story: Purpose of that form is to schedule action
>to occur "not sooner than". It fails on all but one particular case. It
>simply cold not have been tested.
>
>Reboot is considered destructive operation. Therefore we insisted on
>two-step scheduling for this feature. I fixed this one but wonder
>how many other similar features there were recently introduced.
>
>And the final rhetorical question: Have the author even tried it
>before submitting the patch? I tend to doubt it.
>
>Matej
>
>___
>Spacewalk-devel mailing list
>Spacewalk-devel@redhat.com
>https://www.redhat.com/mailman/listinfo/spacewalk-devel

Thanks!  Keep up the great work.
-- 
Lamont Peterson
Sr. Systems Administrator | Unix Systems Operations
Intermountain Healthcare
Office: 801.442.6497


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

Re: [Spacewalk-devel] On quality of patches

2014-01-30 Thread Innes, Duncan 
> -Original Message-
> From: spacewalk-devel-boun...@redhat.com 
> [mailto:spacewalk-devel-boun...@redhat.com] On Behalf Of Stephen Herr
> Sent: 30 January 2014 13:33
> To: spacewalk-devel@redhat.com
> Subject: Re: [Spacewalk-devel] On quality of patches
> 
> On 01/30/2014 07:32 AM, Duncan Mac-Vicar P. wrote:
> >
> > I fully agree with you that this is not acceptable.
> >
> > Now, I think your diagnostic is IMHO neither fair or correct. This 
> > does not have to do with the "community contributions" but with the 
> > setup of the project itself.
> > We sometimes have sent patches that are very strictly reviewed, 
> > sometimes needing change multiple times to get a reviewer happy.
> >
> > Then next day our internal testsuite fails only to realize that 
> > someone with direct commit access did a big refactoring and
committed 
> > very broken code in a big patch that was not reviewed by anyone and 
> > which quality was also obviously not the best. We asked ourselves, 
> > what is the point of reviewing "some of them"?. Not only code but
> > also design decisions and way of approaching solutions.
> >
> > Sometimes this happens with our own patches, depending on the 
> > reviewer, they may get committed faster.
> >
> > The setup of the review process is broken.
> >
> > - All code should be committed in similar units (features/branches)
> > - All code should be able to be reviewed and vetoed by everyone
> >
> > OpenStack has this model working quite successfully. Every patch is 
> > reviewed with +1, and they need a certain amounts of ACKS to get 
> > committed. Everyone can review and people learn in the process, and
> > it is a great source of inspiration for other projects.
> 
> For what it's worth I agree with Duncan. I think that having 
> a consistent process requiring multiple reviews for all 
> changes could only improve code quality and make Spacewalk 
> more accessible to community contributors. The process Duncan 
> describes is very common among modern open source projects 
> and seems to work well.
> 
> -Stpehen
> 

I can't agree more with Duncan on this (at least one Duncan talks
sense!).

Duncan

This message has been checked for viruses and spam by the Virgin Money email 
scanning system powered by Messagelabs.

This e-mail is intended to be confidential to the recipient. If you receive a 
copy in error, please inform the sender and then delete this message.

Virgin Money plc - Registered in England and Wales (Company no. 6952311). 
Registered office - Jubilee House, Gosforth, Newcastle upon Tyne NE3 4PL. 
Virgin Money plc is authorised by the Prudential Regulation Authority and 
regulated by the Financial Conduct Authority and the Prudential Regulation 
Authority.

The following companies also trade as Virgin Money. They are both authorised 
and regulated by the Financial Conduct Authority, are registered in England and 
Wales and have their registered office at Jubilee House, Gosforth, Newcastle 
upon Tyne NE3 4PL: Virgin Money Personal Financial Service Limited (Company no. 
3072766) and Virgin Money Unit Trust Managers Limited (Company no. 3000482).

For further details of Virgin Money group companies please visit our website at 
virginmoney.com

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

Re: [Spacewalk-devel] On quality of patches

2014-01-30 Thread Paul Robert Marino 
There is an other factor here that no one has mentioned. Recently I've found and reported a lot of bugs in the nightly repo. Where they came from is usually some one updates a portion of the code to fix a specific issue but since a portion of that code is called by multiple parts of the web interface it may break other pages than the one the developer is focused on.I think we need a map of what these shared file and functions effect so we can do more formal QA testing in the future-- Sent from my HP Pre3On Jan 30, 2014 7:33, Duncan Mac-Vicar P.  wrote: On 30/01/14 11:57, Matej Kollar wrote:
> Hi all.
> 
> We welcome community contributions and we want to maintain some level
> of quality. Natural expectation is that every proposed patch is
> tested for the functionality.

I fully agree with you that this is not acceptable.

Now, I think your diagnostic is IMHO neither fair or correct. This does
not have to do with the "community contributions" but with the setup of
the project itself.
We sometimes have sent patches that are very strictly reviewed,
sometimes needing change multiple times to get a reviewer happy.

Then next day our internal testsuite fails only to realize that someone
with direct commit access did a big refactoring and committed very
broken code in a big patch that was not reviewed by anyone and which
quality was also obviously not the best. We asked ourselves, what is the
point of reviewing "some of them"?. Not only code but also design
decisions and way of approaching solutions.

Sometimes this happens with our own patches, depending on the reviewer,
they may get committed faster.

The setup of the review process is broken.

- All code should be committed in similar units (features/branches)
- All code should be able to be reviewed and vetoed by everyone

OpenStack has this model working quite successfully. Every patch is
reviewed with +1, and they need a certain amounts of ACKS to get
committed. Everyone can review and people learn in the process, and it
is a great source of inspiration for other projects.

We care about quality and you can see that we not only contributed our
own testsuite in early 2011 with the first release of our own product,
but then focused lot of contributions in having the testsuites enabled
and working again.

But all this is pointless if the process has holes. Right now it depends
on a lot of luck to be useful.

We suggested not long time ago in the mailing list to move to a github
approach where code could be submitted only with pull-requests that need
to be reviewed. Policies could be created that at least 2-3 acks is
needed to merge a feature. People can study those reviews and learn in
the process.
Continuous integration could be setup on top of this process, to have up
to date results.

But what you are seeing, the process is kind of designed to produce
those results, which is a process where code has different ways to
arrive to master, which different quality outcomes.

-- 
Duncan Mac-Vicar P. - http://www.suse.com/

SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix
Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstraße 5, 90409 Nürnberg, Germany

___
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

Re: [Spacewalk-devel] On quality of patches

2014-01-30 Thread Stephen Herr 

On 01/30/2014 07:32 AM, Duncan Mac-Vicar P. wrote:


I fully agree with you that this is not acceptable.

Now, I think your diagnostic is IMHO neither fair or correct. This does
not have to do with the "community contributions" but with the setup of
the project itself.
We sometimes have sent patches that are very strictly reviewed,
sometimes needing change multiple times to get a reviewer happy.

Then next day our internal testsuite fails only to realize that someone
with direct commit access did a big refactoring and committed very
broken code in a big patch that was not reviewed by anyone and which
quality was also obviously not the best. We asked ourselves, what is the
point of reviewing "some of them"?. Not only code but also design
decisions and way of approaching solutions.

Sometimes this happens with our own patches, depending on the reviewer,
they may get committed faster.

The setup of the review process is broken.

- All code should be committed in similar units (features/branches)
- All code should be able to be reviewed and vetoed by everyone

OpenStack has this model working quite successfully. Every patch is
reviewed with +1, and they need a certain amounts of ACKS to get
committed. Everyone can review and people learn in the process, and it
is a great source of inspiration for other projects.


For what it's worth I agree with Duncan. I think that having a 
consistent process requiring multiple reviews for all changes could only 
improve code quality and make Spacewalk more accessible to community 
contributors. The process Duncan describes is very common among modern 
open source projects and seems to work well.


-Stpehen

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

Re: [Spacewalk-devel] On quality of patches

2014-01-30 Thread Duncan Mac-Vicar P. 
On 30/01/14 11:57, Matej Kollar wrote:
> Hi all.
> 
> We welcome community contributions and we want to maintain some level
> of quality. Natural expectation is that every proposed patch is
> tested for the functionality.

I fully agree with you that this is not acceptable.

Now, I think your diagnostic is IMHO neither fair or correct. This does
not have to do with the "community contributions" but with the setup of
the project itself.
We sometimes have sent patches that are very strictly reviewed,
sometimes needing change multiple times to get a reviewer happy.

Then next day our internal testsuite fails only to realize that someone
with direct commit access did a big refactoring and committed very
broken code in a big patch that was not reviewed by anyone and which
quality was also obviously not the best. We asked ourselves, what is the
point of reviewing "some of them"?. Not only code but also design
decisions and way of approaching solutions.

Sometimes this happens with our own patches, depending on the reviewer,
they may get committed faster.

The setup of the review process is broken.

- All code should be committed in similar units (features/branches)
- All code should be able to be reviewed and vetoed by everyone

OpenStack has this model working quite successfully. Every patch is
reviewed with +1, and they need a certain amounts of ACKS to get
committed. Everyone can review and people learn in the process, and it
is a great source of inspiration for other projects.

We care about quality and you can see that we not only contributed our
own testsuite in early 2011 with the first release of our own product,
but then focused lot of contributions in having the testsuites enabled
and working again.

But all this is pointless if the process has holes. Right now it depends
on a lot of luck to be useful.

We suggested not long time ago in the mailing list to move to a github
approach where code could be submitted only with pull-requests that need
to be reviewed. Policies could be created that at least 2-3 acks is
needed to merge a feature. People can study those reviews and learn in
the process.
Continuous integration could be setup on top of this process, to have up
to date results.

But what you are seeing, the process is kind of designed to produce
those results, which is a process where code has different ways to
arrive to master, which different quality outcomes.

-- 
Duncan Mac-Vicar P. - http://www.suse.com/

SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix
Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstraße 5, 90409 Nürnberg, Germany

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

[Spacewalk-devel] On quality of patches

2014-01-30 Thread Matej Kollar 
Hi all.

We welcome community contributions and we want to maintain some level
of quality. Natural expectation is that every proposed patch is
tested for the functionality.

However, recently I came across quite nasty little thing in Spacewalk...

Using SSM to schedule reboot one might end up very unpleasantly surprised,
expecting it not to occur for specified period of time, when in fact it
is being scheduled "as soon as possible", no matter what you try.

(There was no way it ever worked - parameters for forms were not passed
to script in request (where DatePicker looks for them) and new Date
object was returned. Well, someone scamped his work, so I had to do it
for him last night. And I consider myself lucky that I found out...
As en exercise in applied imagination, try to imagine OSAD running on
such machine. To stretch your imagination even further, imagine
the machine acting as a life supporting computer during child surgery...)

Btw: next time have a peek into struts-config.xml and look for
form-bean and form-property tags... say like in
eea3c6320bfc3dae8532844a7c0e71dcc5712c39.

To continue with our story: Purpose of that form is to schedule action
to occur "not sooner than". It fails on all but one particular case. It
simply cold not have been tested.

Reboot is considered destructive operation. Therefore we insisted on
two-step scheduling for this feature. I fixed this one but wonder
how many other similar features there were recently introduced.

And the final rhetorical question: Have the author even tried it
before submitting the patch? I tend to doubt it.

Matej

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