[petsc-dev] Pushing non-working code

[Karl Rupp]

Hi guys,

 >     They issue warnings and the code can't possibly execute correctly.
>     Just don't push it (or push it somewhere else) until it's been
>     cleaned up to the point where it's not wasting our time to review.
>
>
> I would love it if people never pushed code with any bugs. This is
> possible, just not efficient. All changes to
> workflow should be evaluated on this basis. For this particular change,
>
>    1) It does not break the build
>
>    2) Its a new feature, so does not break tests except the ones its
> supposed to
>
> I do think warnings are annoying and people should endeavor to push code
> with no
> warnings, but making this a requirement takes away flexibility while
> providing nothing
> I can see except lack of Jed Annoyance. The claim that you are code
> reviewing this
> push is false on its face.

I support Jed's point at this place. Staying 'close to stability' has 
the additional benefit of finding bugs earlier and being able to go for 
shorter release cycles. Jed only requests a higher discipline when 
committing code. It usually takes a fraction of the time to fix warnings 
immediately rather than keeping them around.


>     Related: I would like to start tweaking our workflow to make
>     petsc-dev more consistently stable, so that more applications can
>     work with it instead of needing to wait for a release. Having people
>     pushing code that doesn't work, isn't tested, and obviously wouldn't
>     pass review is not good for stability.

That's why I want to tackle improved test system output. Once we get 
used to an 'all green' test report in the morning, motivation for 
keeping this state goes up. Also, I think that it improves the 
discipline in adding tests for things not yet covered by the test 
system. That may sound naive, but it is my personal experience with 
other software.

Best regards,
Karli