Re: Notes for DLang maintainers

2017-02-28 Thread Jacob Carlborg via Digitalmars-d

On 2017-02-28 17:37, Nick Sabalausky (Abscissa) wrote:


Maybe they should be, but with the basic git interface, or any front-end
I've seen, they're terribly convoluted. Particularly squashing. Well,
either that, or the docs are just really, REALLY bad.

There's no reason either one of those operations couldn't/shouldn't be a
(*simple*) one-line command, and yet, they just...aren't.


When I google "git squash commits", the first hit is this page [1]. That 
site basically explains what H. S. Teoh said [2] with some more words 
and an example. Next hit is this one [3], which again have the basically 
the same command, just with a more fleshed out explanation of how it 
works. Not sure where you have been reading.


[1] https://ariejan.net/2011/07/05/git-squash-your-latests-commits-into-one/

[2] 
http://forum.dlang.org/post/mailman.782.1488302816.31550.digitalmar...@puremagic.com


[3] 
http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html


--
/Jacob Carlborg


Re: Notes for DLang maintainers

2017-02-28 Thread Nick Sabalausky (Abscissa) via Digitalmars-d

On 02/28/2017 12:18 PM, H. S. Teoh via Digitalmars-d wrote:


Whut...?  What can be easier than:

git checkout my_bug_fix
git rebase -i master
# Editor pops up, change "pick" to "squash" (or "s" for short)
# for every commit you wish to squash.
git push -f origin


1. wit squash my_bug_fix
(Or something along those lines. Not yet implemented.)

2. Not needing to intuit that process from the awful docs and Sheldon 
Cooper-esque web articles: 



Your instruction right up there, honestly, is the first time I've *ever* 
come across any instructions or explanation for "How do I squash my pr" 
that actually makes any straightforward sense.



There's no reason either one of those operations couldn't/shouldn't be
a (*simple*) one-line command, and yet, they just...aren't. But then,
that's git :/ (And no, rebasing a PR is NOT a one-line git command,
and no, add-on scripts don't count towards usability.)


"Add-on scripts" are just user-defined functions.  Is it really that bad
to expect programmers to be able to write their own functions as opposed
to expecting the standard lib to spoonfeed them for every little
conceivable task they might possibly need to accomplish?


When it's a basic common task, then yes.

When it's something that's expected of project contributors, then again, 
yes. (Paths of resistance are a great way to discourage contribution and 
work against the OSS ecosystem.)



(Having said that, though, I agree that your standard average *nix shell
interface is really dumb, and being forced to write scripts in that
broken language can be painful.  One of these days I should write a
shell with D-like syntax that doesn't require memorizing meaningless
30-year-old typographical infelicities like 'grep'.)



Meh, I just use D anymore for any non-trivial scripts ;)


Actually, about a week ago, I finally got around to staring a lib/cli
front-end for git (github support planned, too) to make everything
sane. Too early for anything public though, a lot still incomplete, a
lot that may still be in flux.


Curious to see what you come up with.  AIUI, git is already providing
the 'porcelain' interface by default, so we see today is already far
better than what Linus used to use. (I dread to imagine how one might
use git back then!  I'll bet it's like piloting an airplane by coding in
machine language in real-time. :-P)  Let's see if you can come up with
something even better! ;-)   (Though I'll admit it may be easier than it
sounds. But still... the only sane way to use git is to understand its
innards -- y'know, all that DAG stuff -- 'cos it simply doesn't make any
sense otherwise.)



I highly suspect how much people like my tool will have a strong inverse 
correlation to how well they understand git (and that's fine - if you're 
good with git, then git gets the job done, no problem.)


TBH though, the issues I've had understanding and using git have had far 
less to do with any DAG-iness of git[1], and more with:


A. The details of the CLI just plain being a mess (inconsistencies, 
questionable defaults, reinventing alternate meanings for established 
VCS terminology, etc)


B. Too much catering to "invent your own workflow!" when most users just 
need "Here's best-practice workflow" and tools appropriate for it.


C. Docs that assume more in-depth knowledge than is really necessary, 
and howto's that can't get straight to the point without throwing a 
bunch of blather at you first (again, the Sheldon Cooper teaching style.)


[1] For example, I've never really had any issue dealing with git's 
branches. No 11-tentacle monsters or anything. But then, I don't go 
branch-crazy either (does anyone?): I have "master" and then I have a 
branch for each PR (if I'm submitting PRs). And then once in a rare 
while, if I'm trying something big and experimental, I'll have a branch 
for that experiment.




Re: Notes for DLang maintainers

2017-02-28 Thread H. S. Teoh via Digitalmars-d
On Tue, Feb 28, 2017 at 06:24:45PM +, Russel Winder via Digitalmars-d wrote:
> On Tue, 2017-02-28 at 09:18 -0800, H. S. Teoh via Digitalmars-d wrote:
> > […]
> 
> > 30-year-old typographical infelicities like 'grep'.)
> 
> Surely typing grep is far easier than get_regular_expression_print

It only makes sense to those who already know what it means.  A typical
syndrome of UI's designed by programmers.

It should have been something more immediately descriptive, like 'find'
or 'match'.  But it's too late for that now, as 'find' now has a
different meaning thanks to yet another historical accident.

And don't get me started on `awk` or `sed`. Or `getty`.  As they say,
ASCII stupid question, getty stupid ANSI. :-P


[…]
> > Curious to see what you come up with.  AIUI, git is already
> > providing the 'porcelain' interface by default, so we see today is
> > already far better than what Linus used to use. (I dread to imagine
> > how one might use git back then!  I'll bet it's like piloting an
> > airplane by coding in machine language in real-time. :-P)  Let's see
> > if you can come up with something even better! ;-)   (Though I'll
> > admit it may be easier than it sounds. But still... the only sane
> > way to use git is to understand its innards -- y'know, all that DAG
> > stuff -- 'cos it simply doesn't make any sense otherwise.)
> 
> Despite 10 years of trying to make things better the Git command line
> is still dreadful and the perpetrators should apologies to the world.
> Sadly Mercurial and Bazaar have lost (thought they are still used), and
> Git is the winner. Programmers just have to use Git. Despite being crap
> the Git command line can just about be mastered enough to be used, and
> programmers should just do that.

I make no defense for the weirdness of git's current interface.  There's
a lot of asymmetry and unexpected exceptions, that only make sense in
retrospect (and sometimes not even that). Such as `git checkout -b` for
creating a new branch, but `git branch -d` for deleting a branch. Or
`git checkout` for *both* replacing a file with the last committed
version and for switching branches.

But that's what you get for an interface that's evolved rather than
designed.


> Blogs showing cute tricks with Git, or anything involving ^^^HEAD
> should be totally ignored by all right thinking people. The trick is
> to use the simple stuff, use it well, and ignore all the tricks, and
> all the people getting high on cute tricks.
[...]

Actually, all expectation of git being anything even remotely resembling
a traditional version control system is not thinking right. The only
sane way to make any sense of it whatsoever is to think on *its* terms,
i.e., think in terms of DAG manipulations.  Trying to impose (or think
in terms of) foreign concepts inherited from traditional VCS's
inevitably leads to frustration, annoyance, and hair loss.

For instance, the only way git "branches" even begin to make any sense
at all is when you think of them as mere pointers to certain DAG nodes,
rather than branches in the traditional sense of the word.  I.e., as
pure DAG manipulations and nothing more. Attaching any further meaning
to it which isn't there only leads to confusion and incomprehension.
(And usually also hair/code loss or your DAG turning into an
11-tentacled monster on you.)

In that sense, it's a *good* thing the git interface is so weird: it
forces you to confront the fact that you're dealing not with a VCS (in
the traditional sense of the word), but with a DAG manipulation system.
One that happens to have useful idioms that resembles a VCS of sorts,
but nonetheless at its heart still a mere DAG manipulation system.
Trying to use it as anything else or in any other way amounts to nothing
more than a cargo cult, with about the same level of effectiveness.


T

-- 
Turning your clock 15 minutes ahead won't cure lateness---you're just making 
time go faster!


Re: Notes for DLang maintainers

2017-02-28 Thread Russel Winder via Digitalmars-d
On Tue, 2017-02-28 at 09:18 -0800, H. S. Teoh via Digitalmars-d wrote:
> […]

> 30-year-old typographical infelicities like 'grep'.)

Surely typing grep is far easier than get_regular_expression_print
> 
[…]
> Curious to see what you come up with.  AIUI, git is already providing
> the 'porcelain' interface by default, so we see today is already far
> better than what Linus used to use. (I dread to imagine how one might
> use git back then!  I'll bet it's like piloting an airplane by coding
> in
> machine language in real-time. :-P)  Let's see if you can come up
> with
> something even better! ;-)   (Though I'll admit it may be easier than
> it
> sounds. But still... the only sane way to use git is to understand
> its
> innards -- y'know, all that DAG stuff -- 'cos it simply doesn't make
> any
> sense otherwise.)

Despite 10 years of trying to make things better the Git command line
is still dreadful and the perpetrators should apologies to the world.
Sadly Mercurial and Bazaar have lost (thought they are still used), and
Git is the winner. Programmers just have to use Git. Despite being crap
the Git command line can just about be mastered enough to be used, and
programmers should just do that.

Blogs showing cute tricks with Git, or anything involving ^^^HEAD
should be totally ignored by all right thinking people. The trick is to
use the simple stuff, use it well, and ignore all the tricks, and all
the people getting high on cute tricks.

But I rant.

-- 
Russel.
=
Dr Russel Winder  t: +44 20 7585 2200   voip: sip:russel.win...@ekiga.net
41 Buckmaster Roadm: +44 7770 465 077   xmpp: rus...@winder.org.uk
London SW11 1EN, UK   w: www.russel.org.uk  skype: russel_winder

signature.asc
Description: This is a digitally signed message part


Re: Notes for DLang maintainers

2017-02-28 Thread H. S. Teoh via Digitalmars-d
On Tue, Feb 28, 2017 at 11:37:45AM -0500, Nick Sabalausky (Abscissa) via 
Digitalmars-d wrote:
> On 02/28/2017 02:37 AM, Jacob Carlborg wrote:
> > On 2017-02-28 00:42, Nick Sabalausky (Abscissa) wrote:
> > 
> > > Contributors shouldn't have to know as much about git as a
> > > project's maintainers. So these features, if used, are AWESOME.
> > 
> > Squashing and rebasing is part of the basic git, in my opinion.
> > 
> 
> Maybe they should be, but with the basic git interface, or any
> front-end I've seen, they're terribly convoluted. Particularly
> squashing. Well, either that, or the docs are just really, REALLY bad.
[...]

Whut...?  What can be easier than:

git checkout my_bug_fix
git rebase -i master
# Editor pops up, change "pick" to "squash" (or "s" for short)
# for every commit you wish to squash.
git push -f origin


> There's no reason either one of those operations couldn't/shouldn't be
> a (*simple*) one-line command, and yet, they just...aren't. But then,
> that's git :/ (And no, rebasing a PR is NOT a one-line git command,
> and no, add-on scripts don't count towards usability.)

"Add-on scripts" are just user-defined functions.  Is it really that bad
to expect programmers to be able to write their own functions as opposed
to expecting the standard lib to spoonfeed them for every little
conceivable task they might possibly need to accomplish?

(Having said that, though, I agree that your standard average *nix shell
interface is really dumb, and being forced to write scripts in that
broken language can be painful.  One of these days I should write a
shell with D-like syntax that doesn't require memorizing meaningless
30-year-old typographical infelicities like 'grep'.)


> Actually, about a week ago, I finally got around to staring a lib/cli
> front-end for git (github support planned, too) to make everything
> sane. Too early for anything public though, a lot still incomplete, a
> lot that may still be in flux.

Curious to see what you come up with.  AIUI, git is already providing
the 'porcelain' interface by default, so we see today is already far
better than what Linus used to use. (I dread to imagine how one might
use git back then!  I'll bet it's like piloting an airplane by coding in
machine language in real-time. :-P)  Let's see if you can come up with
something even better! ;-)   (Though I'll admit it may be easier than it
sounds. But still... the only sane way to use git is to understand its
innards -- y'know, all that DAG stuff -- 'cos it simply doesn't make any
sense otherwise.)


T

-- 
Don't modify spaghetti code unless you can eat the consequences.


Re: Notes for DLang maintainers

2017-02-28 Thread Vladimir Panteleev via Digitalmars-d
On Tuesday, 28 February 2017 at 15:49:56 UTC, Andrei Alexandrescu 
wrote:
There can be any amount of discussion about this, to no 
conclusive results, and any argument may be responded with "I 
don't buy that". That's simply because, again, there's some 
subjective factor and there is no perfect approach that proves 
the others wrong. To wit, I have stated my argument several 
times in public and private but somehow it did not count.


Apologies if I have forgotten about any important arguments from 
past discussions; however, I can't recall any substantial ones 
that would settle this argument.


Also, obviously the Internet can be used to find support for 
anything, and with that caveat allow me:


* "Why does every serious Github repo I do pull requests for 
want me to squash my commits into a single commit?" 
http://softwareengineering.stackexchange.com/questions/263164/why-squash-git-commits-for-pull-requests


The top answer advocates not squashing the commits when it 
doesn't make sense to.


* "I’ve been contributing to more projects in which it is 
expected that I should submit a pull request that contains a 
single commit." 
http://ndlib.github.io/practices/one-commit-per-pull-request/


This article was brought up here before, with a similar debate.

* "Squashing your branch's changes into one commit is "good 
form" and helps the person merging your request to see 
everything that is going on." 
https://github.com/projecthydra/hydra-head/blob/master/CONTRIBUTING.md


Only suggests considering whether squashing is appropriate. Links 
to the previous article.


* "But one thing occasionally bothers me, and that's pull 
requests that come loaded with several temporary commits. I 
often find myself asking contributors to squash those into a 
single descriptive commit." 
http://eli.thegreenplace.net/2014/02/19/squashing-github-pull-requests-into-a-single-commit


Mentions that the steps are for dealing with "temporary commits". 
Does not advocate one style over the other.


Perhaps there is some misunderstanding on what the point of 
disagreement is?


- Splitting changes across multiple pull requests is a point 
orthogonal to this one. I do wish we would adopt a workflow where 
larger pull requests are acceptable as long as the changes are 
properly divided into commits, as I believe it would be overall 
more productive; however, this is not what this thread is about.


- Squashing temporary commits is always good. We are both in 
agreement here.


- What we seem to disagree on is whether commits should be 
squashed in other cases. You argue that if a pull request 
contains enough changes that they could be split across several 
commits, they should be split across several pull requests. I am 
not contesting this argument (at least in this discussion). 
However, you also seem to say that pull requests containing 
enough changes that they could be split across several commits 
should be squashed into a single commit before merging, which is 
what I can't comprehend.


To reiterate, squashing commits that do not need to be squashed 
destroys information (commit messages and separation of changes) 
and makes bisecting, blaming and generally examining git history 
more difficult. Squashing discourages splitting changes into 
granular commits (even smaller than would make sense for a PR), 
and documenting each in part. Writing detailed commit messages is 
really important - not only does it provide additional context to 
people not familiar with the code base in question, it also 
forces the author to review their justification for the changes 
and consider alternative solutions.


What does squashing achieve, on the practical front?

Vladimir, I am gathering that this makes you unhappy and are 
close to getting ultimative about this.


What I'm unhappy about is that waste so much time arguing about 
things that seem to have an obvious correct answer, and when I 
ask for rationales for your decisions, I get huge replies about 
politics and what Facebook does and how these discussions don't 
scale and zero practical arguments to support your point. This is 
a systematic problem and has happened several times before.


Where are the practical arguments?

Closest thing is link number 2. Quoting:

For pull requests, a single commit is easier to inspect, 
critique, and discuss. By creating a single commit, I am saying 
“This is a logical unit of work” for the project. I can explain 
what and why these changes are made - developer documentation 
if you will. At a future date, for other contributors, it is 
easier to get a context for a small change in one file if that 
change tracks to a larger unit of work.


Exactly the same arguments apply to each commit in a commit 
series which should not be squashed.


Where is the rationale for squashing self-contained commits with 
well-written commit messages over not doing so?


* Any organization, be it for profit (company) or not 
(university, oss project) has 

Re: Notes for DLang maintainers

2017-02-28 Thread Nick Sabalausky (Abscissa) via Digitalmars-d

On 02/28/2017 02:37 AM, Jacob Carlborg wrote:

On 2017-02-28 00:42, Nick Sabalausky (Abscissa) wrote:


Contributors shouldn't have to know as much about git as a project's
maintainers. So these features, if used, are AWESOME.


Squashing and rebasing is part of the basic git, in my opinion.



Maybe they should be, but with the basic git interface, or any front-end 
I've seen, they're terribly convoluted. Particularly squashing. Well, 
either that, or the docs are just really, REALLY bad.


There's no reason either one of those operations couldn't/shouldn't be a 
(*simple*) one-line command, and yet, they just...aren't. But then, 
that's git :/ (And no, rebasing a PR is NOT a one-line git command, and 
no, add-on scripts don't count towards usability.)


Actually, about a week ago, I finally got around to staring a lib/cli 
front-end for git (github support planned, too) to make everything sane. 
Too early for anything public though, a lot still incomplete, a lot that 
may still be in flux.




Re: Notes for DLang maintainers

2017-02-28 Thread Andrei Alexandrescu via Digitalmars-d

On 02/28/2017 11:03 AM, Seb wrote:

On Tuesday, 28 February 2017 at 14:52:36 UTC, Andrei Alexandrescu wrote:

Thanks. I'd replace "changes should be split into as many commits as
is reasonable" with "changes should be split into as many pull
requests as is reasonable", which is a natural consequence of "most
pull requests should consist of one commit upon merging". (Of course
there may be several commits during PR review.)

One vs. several commits per merged pull request is a matter in which
reasonable people may disagree, and we can't do both ways. The
Foundation fosters that github pull requests are squashed upon
merging, with exceptions that need to be justified.
BTW we should put this in our guidelines, we have
https://wiki.dlang.org/Contributing_to_Phobos but not an equivalent
covering all of dlang properties. Is there one?


Not that I would know of.
I started an initial wiki entry (basically a cleaned copy/paste version
of my post):

https://wiki.dlang.org/Guidelines_for_maintainers

@other maintainers: As this seems to be quite useful for newcomers and
semi-regular contributors, please help to extend this wiki entry (or
whenever a question pops up on GH).


That's awesome. Thanks! -- Andrei


Re: Notes for DLang maintainers

2017-02-28 Thread Seb via Digitalmars-d
On Tuesday, 28 February 2017 at 14:52:36 UTC, Andrei Alexandrescu 
wrote:
Thanks. I'd replace "changes should be split into as many 
commits as is reasonable" with "changes should be split into as 
many pull requests as is reasonable", which is a natural 
consequence of "most pull requests should consist of one commit 
upon merging". (Of course there may be several commits during 
PR review.)


One vs. several commits per merged pull request is a matter in 
which reasonable people may disagree, and we can't do both 
ways. The Foundation fosters that github pull requests are 
squashed upon merging, with exceptions that need to be 
justified.
BTW we should put this in our guidelines, we have 
https://wiki.dlang.org/Contributing_to_Phobos but not an 
equivalent covering all of dlang properties. Is there one?


Not that I would know of.
I started an initial wiki entry (basically a cleaned copy/paste 
version of my post):


https://wiki.dlang.org/Guidelines_for_maintainers

@other maintainers: As this seems to be quite useful for 
newcomers and semi-regular contributors, please help to extend 
this wiki entry (or whenever a question pops up on GH).


Re: Notes for DLang maintainers

2017-02-28 Thread Andrei Alexandrescu via Digitalmars-d

On 02/28/2017 10:04 AM, Vladimir Panteleev wrote:

On Tuesday, 28 February 2017 at 14:52:36 UTC, Andrei Alexandrescu wrote:

Thanks. I'd replace "changes should be split into as many commits as
is reasonable" with "changes should be split into as many pull
requests as is reasonable", which is a natural consequence of "most
pull requests should consist of one commit upon merging". (Of course
there may be several commits during PR review.)


Well... not always. For example, introducing a private function that is
not called from anywhere is something that doesn't really make sense as
a pull request of its own, but does make sense as a separate commit.


One vs. several commits per merged pull request is a matter in which
reasonable people may disagree, and we can't do both ways. The
Foundation fosters that github pull requests are squashed upon
merging, with exceptions that need to be justified.


Sorry, but I don't think that's reasonable at all.

I have seen no arguments to support this way of doing things, only
downsides. Established major projects seem to agree.

As far as I can see, this is not about a subjective point regarding
which reasonable people may disagree. It seems to be a poorly justified
mandate, that's all.

As I've mentioned previously, you will need to provide some arguments
which would outweigh those supporting the opposite position.


There can be any amount of discussion about this, to no conclusive 
results, and any argument may be responded with "I don't buy that". 
That's simply because, again, there's some subjective factor and there 
is no perfect approach that proves the others wrong. To wit, I have 
stated my argument several times in public and private but somehow it 
did not count.


Here we are simply applying a way of doing things that has worked at 
Facebook over six years and two orders of magnitude increase in team 
size. (They use phabricator there, which has a similar workflow.) It is 
obvious to me that other organizations use similar approaches; and also 
that other organizations use different approaches, also with good 
results. I should add that Facebook is one of the most successful 
software companies in history, and that may count for something.


Also, obviously the Internet can be used to find support for anything, 
and with that caveat allow me:


* "Why does every serious Github repo I do pull requests for want me to 
squash my commits into a single commit?" 
http://softwareengineering.stackexchange.com/questions/263164/why-squash-git-commits-for-pull-requests


* "I’ve been contributing to more projects in which it is expected that 
I should submit a pull request that contains a single commit." 
http://ndlib.github.io/practices/one-commit-per-pull-request/


* "Squashing your branch's changes into one commit is "good form" and 
helps the person merging your request to see everything that is going 
on." https://github.com/projecthydra/hydra-head/blob/master/CONTRIBUTING.md


* "But one thing occasionally bothers me, and that's pull requests that 
come loaded with several temporary commits. I often find myself asking 
contributors to squash those into a single descriptive commit." 
http://eli.thegreenplace.net/2014/02/19/squashing-github-pull-requests-into-a-single-commit


I'll stop here. Sure enough, there'd be other links to pages describing 
how many commits per PR work fine. Our organization happens do things 
this particular way, and it's not the only one. Moreover, we are are not 
rigidly enforcing it; within reason, definitely leeway exists.


Vladimir, I am gathering that this makes you unhappy and are close to 
getting ultimative about this. I have made all attempts I could to 
publicly and privately explain matters. I kindly ask you to consider the 
following:


* Any organization, be it for profit (company) or not (university, oss 
project) has certain rules in place. In all likelihood not all 
participants would choose the exact same rules if they could, yet they 
choose to walk (factually or virtually) in there every day and 
contribute to the organization. A simple way to look at things would be, 
would you quit a job or start a protracted argument with the CEO if the 
coding guidelines contained "please squash your github commits"?


* The leader of an organization (in this case me) cannot afford the time 
to justify and debate every single minor decision they make. No 
organization works that way. I am under enormous pressure to bring money 
in the Foundation, organize DConf, mentor students, write articles, 
contribute code, all while keeping things running.


* You seem to frame things as an arbitrary imposition made by an 
oppressive leadership. Think of it - ever since we have worked together 
(your first post is dated 2007-01-06!), is this the pattern by which 
matters have been conducted? Do you think it would be a fair 
characterization of Walter and my philosophy and values?


The short of it is we simply cannot work this way, as a 

Re: Notes for DLang maintainers

2017-02-28 Thread Vladimir Panteleev via Digitalmars-d
On Tuesday, 28 February 2017 at 14:52:36 UTC, Andrei Alexandrescu 
wrote:
Thanks. I'd replace "changes should be split into as many 
commits as is reasonable" with "changes should be split into as 
many pull requests as is reasonable", which is a natural 
consequence of "most pull requests should consist of one commit 
upon merging". (Of course there may be several commits during 
PR review.)


Well... not always. For example, introducing a private function 
that is not called from anywhere is something that doesn't really 
make sense as a pull request of its own, but does make sense as a 
separate commit.


One vs. several commits per merged pull request is a matter in 
which reasonable people may disagree, and we can't do both 
ways. The Foundation fosters that github pull requests are 
squashed upon merging, with exceptions that need to be 
justified.


Sorry, but I don't think that's reasonable at all.

I have seen no arguments to support this way of doing things, 
only downsides. Established major projects seem to agree.


As far as I can see, this is not about a subjective point 
regarding which reasonable people may disagree. It seems to be a 
poorly justified mandate, that's all.


As I've mentioned previously, you will need to provide some 
arguments which would outweigh those supporting the opposite 
position.



The Foundation fosters


IMHO, this phrase does not belong in technical discussions.



Re: Notes for DLang maintainers

2017-02-28 Thread Andrei Alexandrescu via Digitalmars-d

On 02/28/2017 08:48 AM, Vladimir Panteleev wrote:

On Tuesday, 28 February 2017 at 13:10:17 UTC, Andrei Alexandrescu wrote:

This would be the overwhelmingly frequent case.
...
This indicates a problem with the PR more often than not.


It is unfortunate that these two seem to be true right now, so given
that, I'll agree with you. Currently a lot of contributed code seems to
be placed in unnecessarily large commits with minimalist commit
messages, which would rank lather low on the quality scale of large
established projects. Ideally changes should be split into as many
commits as is reasonable, which by itself brings a lot of benefits, and
described in detail (50-char summary, long description of the change,
and change rationale). For examples, see the commit message guidelines
of Linux, git, or really any project that's large enough to have commit
message guidelines.


Thanks. I'd replace "changes should be split into as many commits as is 
reasonable" with "changes should be split into as many pull requests as 
is reasonable", which is a natural consequence of "most pull requests 
should consist of one commit upon merging". (Of course there may be 
several commits during PR review.)


One vs. several commits per merged pull request is a matter in which 
reasonable people may disagree, and we can't do both ways. The 
Foundation fosters that github pull requests are squashed upon merging, 
with exceptions that need to be justified.


BTW we should put this in our guidelines, we have 
https://wiki.dlang.org/Contributing_to_Phobos but not an equivalent 
covering all of dlang properties. Is there one?



Thanks,

Andrei





Re: Notes for DLang maintainers

2017-02-28 Thread Vladimir Panteleev via Digitalmars-d
On Tuesday, 28 February 2017 at 13:10:17 UTC, Andrei Alexandrescu 
wrote:

This would be the overwhelmingly frequent case.
...
This indicates a problem with the PR more often than not.


It is unfortunate that these two seem to be true right now, so 
given that, I'll agree with you. Currently a lot of contributed 
code seems to be placed in unnecessarily large commits with 
minimalist commit messages, which would rank lather low on the 
quality scale of large established projects. Ideally changes 
should be split into as many commits as is reasonable, which by 
itself brings a lot of benefits, and described in detail (50-char 
summary, long description of the change, and change rationale). 
For examples, see the commit message guidelines of Linux, git, or 
really any project that's large enough to have commit message 
guidelines.




Re: Notes for DLang maintainers

2017-02-28 Thread Andrei Alexandrescu via Digitalmars-d

On 02/28/2017 01:07 AM, Vladimir Panteleev wrote:

On Sunday, 26 February 2017 at 14:38:03 UTC, Seb wrote:

1) Commit squashing


Reminder: please only do this only when it makes sense to (one commit
with significant changes followed by fixup commits that have no
significance on their own).


This would be the overwhelmingly frequent case. Please reach to squash 
commits as the default, unless there is a special case. Thanks.



If the PR already has multiple commits split
into logical changes, don't squash as it makes bisecting and inspecting
history more difficult. If in doubt, do a regular merge.


This indicates a problem with the PR more often than not. Good PRs focus 
on one issue and one issue only.


Of course there would be exceptions. Rules are not a substitute for 
judgment.



Thanks,

Andrei



Re: Notes for DLang maintainers

2017-02-27 Thread Jacob Carlborg via Digitalmars-d

On 2017-02-28 00:42, Nick Sabalausky (Abscissa) wrote:


Contributors shouldn't have to know as much about git as a project's
maintainers. So these features, if used, are AWESOME.


Squashing and rebasing is part of the basic git, in my opinion.

--
/Jacob Carlborg


Re: Notes for DLang maintainers

2017-02-27 Thread Vladimir Panteleev via Digitalmars-d

On Sunday, 26 February 2017 at 14:38:03 UTC, Seb wrote:

1) Commit squashing


Reminder: please only do this only when it makes sense to (one 
commit with significant changes followed by fixup commits that 
have no significance on their own). If the PR already has 
multiple commits split into logical changes, don't squash as it 
makes bisecting and inspecting history more difficult. If in 
doubt, do a regular merge.




Re: Notes for DLang maintainers

2017-02-27 Thread Nicholas Wilson via Digitalmars-d

On Sunday, 26 February 2017 at 14:38:03 UTC, Seb wrote:
As it's getting a bit exhaustive to repeat these bits on GitHub 
over and over again,
I though I summarize a couple of notes that hopefully are 
interesting for the DLang maintainers.


[...]


This is great, please put this somewhere (the wiki?) where it 
won't get lost.


Re: Notes for DLang maintainers

2017-02-27 Thread Nick Sabalausky (Abscissa) via Digitalmars-d

On 02/26/2017 09:38 AM, Seb wrote:>

Review workflow (squashed commits & write access to PRs)


Hoora for these *Please* lets make use of these.

Pretty much everything about the old way couldn't have been any better 
at dissuading regular contributors if it had be DESIGNED to discourage 
contributions. The back and forth on style or other nitpicks, getting 
the dreaded "looks good, plz rebase and squash" thus triggering "shit, 
ok, how the fuck do I 'rebase a pr' again without bugging the reviewer?" 
And as for squash, well that's just such a badly-documented convoluted 
mess in git, every single time I've attempted I gave up and found it 
vastly quicker and easier to just make a fresh branch in a new working 
directory and file-copy the final versions there. Used to even do a new 
PR until I learned enough about git to work out force-pushing the fresh 
new branch to the old PR's remote branch instead of just making a new PR 
(which contributors really shouldn't freaking have to figure out).


Contributors shouldn't have to know as much about git as a project's 
maintainers. So these features, if used, are AWESOME.




Re: Notes for DLang maintainers

2017-02-27 Thread Dukc via Digitalmars-d

On Sunday, 26 February 2017 at 14:38:03 UTC, Seb wrote:

[snip]


Excellent, this info will be useful!


- All commits get squashed into one commit before the merge
- This is enabled for all DLang repos
- "auto-merge-squash" does squashing as auto-merge behavior


I especially liked this one. No squashing just for squashing's 
sake.





Notes for DLang maintainers

2017-02-26 Thread Seb via Digitalmars-d
As it's getting a bit exhaustive to repeat these bits on GitHub 
over and over again,
I though I summarize a couple of notes that hopefully are 
interesting for the DLang maintainers.


Please take this as a friendly summary and personal advice of 
most GH-related process improvements that have happened over the 
last three months. It's (mostly) not intended as a rule book.



There's a (new) formal "Approve" button
===

- GH formalized the review process
- Please use a the "approve" feature instead of LGTM comment

This is important, because all PRs require an approval!
So please approve before you auto-merge (it won't work otherwise).

In the same way GH also allows to attach a "request for changes" 
on a PR.
-> If you have a serious remark, please use the "request a 
change" feature instead of a plain comment as these "request" 
will be nicely shown as warning at the end of a PR (GH will even 
block the merge of a PR until the criticized bits are fixed). 
Moreover "changes requested" will also be shown on the summary of 
all PRs and helps others when they browse the PR list.


Review workflow (squashed commits & write access to PRs)


The ideal workflow is that a PR gets commits appended until its 
final approval, s.t. you only need to review the added changes.


GitHub has two new features to help us here:

1) Commit squashing
---

- All commits get squashed into one commit before the merge
- This is enabled for all DLang repos
- "auto-merge-squash" does squashing as auto-merge behavior

More infos: https://github.com/blog/2141-squash-your-commits

(Before you ask: Yes, CyberShadow had some initial concerns [1] 
about this feature, but we were able to address them and digger 
didn't break)


[1] 
http://forum.dlang.org/post/mciqgandxypjwblex...@forum.dlang.org


2) Write access to PRs
--

- This is an _awesome_ feature that hasn't been used much so far
- It allows maintainer to do those nitpicks themselves (squashing 
all commits, fixing typos, ...) instead of going with the usual 
ping-pong cycle

- It's enabled by default for new PRs
- If someone turned it accidentally off, it's really okay to ask 
him/her as this is a massive time saver


I can only recommend to add the following alias to your 
`~/.gitconfig`:


```
[alias]
  pr  = "!f() { git fetch -fu ${2:-upstream} 
refs/pull/$1/head:pr/$1 && git checkout pr/$1; }; f"

```

With this you can checkout a PR like this:


git pr 5150


And in case you are as lazy as I am and don't want to enter the 
branch to push manually, you can use my small snippet:


```
#!/bin/bash
tmpfile=$(mktemp)
repoSlug=$(git remote -v | grep '^upstream' | head -n1 | perl 
-lne 's/github.com:?\/?(.*)\/([^.]*)([.]git| )// or next; print 
$1,"/",$2')

prNumber=$(git rev-parse --abbrev-ref HEAD | cut -d/ -f 2)
curl -s 
https://api.github.com/repos/${repoSlug}/pulls/${prNumber} > 
$tmpfile

trap "{ rm -f $tmpfile; }" EXIT
headRef=$(cat $tmpfile | jq -r '.head.ref')
headSlug=$(cat $tmpfile | jq -r '.head.repo.full_name')
git push -f g...@github.com:${headSlug} HEAD:${headRef}
```

For more details on pushing changes to a PR, please see this 
article:


More infos: 
https://help.github.com/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork


@all: I recently discovered the nice pattern to prepend sth. like 
"[SQUASH]" to appended commit messages. This helps maintainers as 
well ;-)


Auto-Merge
==

In case you were wondering about the new "auto-merge" and 
"auto-merge-squash" labels.
This is the new auto-merge system that takes the status of all 
required CIs into account (the auto-tester tries the merge after 
its test passed, but doesn't look out for other CIs.)
You can toggle a PR for auto-merge by simply adding this label or 
for the keyboard-enthusiasts:

press "l", press "a" and hit enter.

Warning: this new auto-merge system is officially still WIP 
because:
- "auto-merge"-labelled PRs aren't yet prioritized on the 
auto-tester [1]

- We would like to set _all_ CIs to enforced

For more details, please see [2]:

[1] https://github.com/dlang-bots/dlang-bot/pull/50
[2] https://github.com/dlang-bots/dlang-bot#auto-merge-wip


CI status
=

Martin and I have worked quite a bit on making the CIs more 
reliable. If you see a transient error, please let us know!
In any other case, the "red cross" on a CI has a meaning - 
surprise!

Here's a small summary of the tasks of the "new" CIs:

1) CircleCi

- Tests are run with code coverage enabled
- Phobos only: Over the last weeks, we have tried to unify Phobos 
to share a more common coding style. The CI is in place to ensure 
this status quo for the future
- Phobos only: All unittest blocks are separated from the main 
file and compiled separately. This helps to ensure that the 
examples on dlang.org that are runnable and don't miss any 
imports.


It should be fairly