On Mon, Sep 30, 2013 at 8:32 AM, Andrs, David <[email protected]> wrote:
> On Sun, Sep 29, 2013 at 3:07 PM, Barry Smith <[email protected]> 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: > Sorry, too fast. Here is the script: branch_name=$(git symbolic-ref -q HEAD) if [ "$branch_name" == "refs/heads/maint" ] || [ "$branch_name" == "refs/heads/next" ] then echo "Trying to commit to into " $branch_name ". This is normally not desireable." exit 1 fi exit 0 > > > >> 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. >> > Not quite sure how to prevent that branching off of next. The rest of the checks can probably go into pre-commit hook as well. The only problem is that it is a client side script, so it will not get cloned from the central repo (AFAIK). HTH, David > >> 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 <[email protected]> wrote: >> >> > Barry Smith <[email protected]> writes: >> > >> >>> On the Reported-by tags, could you do them in a structured way as >> >>> >> >>> Reported-by: Matteo Parsani <[email protected]> >> >>> Reported-by: Lisandro Dalcin <[email protected]> >> >> >> >> 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. >> >> >
