[petsc-dev] [petsc-users] MatMPIBAIJSetPreallocationCSR i and j indices

[Andrs, David]

On Sun, Sep 29, 2013 at 3:07 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:

>
>   patch-review doesn't happen. Aside from you, none of the rest of us give
> a shit about patch review hence we should automatically check for whatever
> we can to make up for the lack of review.
>
>   Lots of lame excuses why standards shouldn't be enforced automatically.
> Maybe we should work exclusively inside a IDE that can enforce all kinds of
> rules without security concerns etc etc or we could do development of each
> project in its own VM and thus "make normal commands behave differently
> inside the VM" without security fears. I don't buy these arguments. Our IDE
> is Emacs+shell+make+python and we can make that IDE help us out, it won't
> be perfect but if blocks 80% of the goof ups that is a great thing.
>
>   There is also an enormous psychological/sociological advantage to "the
> development system" enforcing standards: people hate it when after days or
> weeks of working some wise-ass reviewer says your code doesn't adhere to
> standards and they will likely push less code in the future but when an
> automatic test rejects some code and tells the user how to fix it that
> doesn't make them less likely to push code in the future.  Linus has surely
> lost many decent developers over the years by being an asshole, we don't
> need to lose the few we have.  I'm not saying there should be no code
> review, just that the sooner in the process each problem is found the
> better for productively and I see no reason why each git repository
> couldn't have in it somewhere a list of rules for that project that git
> verifies are being followed as the user commits code, pushes code, edits
> code etc. Many of these could just be warnings, for example
>
> commit -a directly into maint, next, or master:  "Are you sure you want to
> do this COMMIT DIRECTLY INTO maint, normally this is not desirable?"
>

​If you put this little "script" into .git/hooks/pre-commit, it will do it:
​



> branch off of next:  "Are you sure you want to branch off of NEXT?"
> commit a file with // in it.  "Yoo Mark, PETSc requires the silly old
> fashion /* comments, you cannot commit this file since you used the modern
> //!"
> variables declared in the middle of source code. "Yoo, we use C89 here,
> make sure all variables are declared at the top of scope!"
> commit a file with hundreds of lines of commented out source. "Don't be
> putting commented out code in the repository!"
> etc.
>
> Going back to my maint screw up today. Just one useful warning from
> git/"our Emacs/shell/make/python IDE" along the way and we would have a
> nice branch we could continue to improve on, instead we have this ugly
> illegal commit directly into maint that will be harder to work with. Yes,
> you can easily say, "that Barry is an idiot who always fucks up procedure"
> but that doesn't help resolve the problem. One damn warning and it won't
> have happened, but no, you refuse to consider having any enforcement
> mechanisms at all and expect all developers to internalize 100% all these
> rules.
>
>    Barry
>
>
>
>
>
> On Sep 29, 2013, at 3:01 PM, Jed Brown <jedbrown at mcs.anl.gov> wrote:
>
> > Barry Smith <bsmith at mcs.anl.gov> writes:
> >
> >>> On the Reported-by tags, could you do them in a structured way as
> >>>
> >>> Reported-by: Matteo Parsani <parsani.matteo at gmail.com>
> >>> Reported-by: Lisandro Dalcin <dalcinl at gmail.com>
> >>
> >>   This is fine, but there needs to be a mechanism to ensure this,
> >
> > We already have whitespace conventions, a convention that there are no
> > memory leaks, a convention that type-specialized functions use dynamic
> > composition, a convention that we do not create dependency loops, a
> > function naming convention, a convention about avoiding globals, a
> > branch usage convention, etc.  None of these are enforced and only some
> > have automated checking.
> >
> >>   imposing requirements on developers that do not have enforcement
> >>   mechanisms (besides being yelled at later) don't work.
> >
> > Patch review is more flexible than any automated tool can be.  It is
> > only time to write an automated tool when the effort to do so, and the
> > attainable accuracy, is a better use of human time than had the same
> > time been devoted to patch review.  Note that patch review can catch
> > many things, but automated tools can only catch that which has been
> > automated.
> >
> >>   Note that this is the same kind of situation as me accidentally
> >>   editing directly into maint.  I did
> >>
> >> git branch barry/fix-matmpibaijsetpreallocationcsr
> >
> > This creates a branch without checking it out.  If you want to create
> > the branch _and_ check it out, use
> >
> > $ git checkout -b barry/fix-matmpibaijsetpreallocationcsr
> >
> >> happily edited away and tested
> >> git commit -a
> >> git push
> >>
> >> Now what should have happened is as soon as I tried to happily edit
> >> away, something should come back and said to me: "are you sure you
> >> want to edit in maint?"
> >
> > This is policy on the branch rather than the fact that you created a new
> > branch prior to starting this work.  (I do that sometimes so that I'll
> > be able to easily compare to the old state later.)  We could perhaps
> > prevent direct commits on 'maint'.
> >
> >> And don't go and mail me some elisp that I can stick in somewhere to
> >> warn me about the maint editing business; that would only solve the
> >> problem for me, not for the n-1 other people in the world who make the
> >> same mistake. Git needs some way of "baking in" a set of standards
> >> (optional obviously) that it does its best to enforce and not leaving
> >> it up to each user to remember 324 things they need to constantly be
> >> checking depending on what project they are working on.
> >
> > There is a security problem with making normal commands behave
> > differently when run inside some repository that you just cloned.  Any
> > such policy would need to be opt-in.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20130930/1ff8ace5/attachment.html>