Inclusive naming, part II #734
Closed
+97
−97
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In an ongoing effort to avoid non-inclusive language, let's avoid using the branch name "master" in a code comment. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
/submit |
|
Submitted as pull.734.git.1600279853.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -81,24 +81,24 @@ sub copy_stdio { | |||
| die "usage: test-terminal program args"; | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In the ongoing effort to make the Git project a more inclusive place,
> let's try to avoid names like "master" where possible.
The two sides of a PTY are called 'master' and 'slave', and I
understand there is a push to move away from these words, but
calling one side with an invented name that is used by nobody else
in the context of talking about PTY, while keeping the word used to
call the other side, would lead to confusion.
A better change is to drop "master_" altogether without replacing
the word with anything, and call them just "input", "output" and
"error". For those who implement the PTY, calling both ends of the
pipe 'master' vs 'slave' may be useful for them, as they are terms
of art they are used to think with. But as end-users of PTY, the
fact that we are holding the 'master' end, and access to the 'slave'
end is done by asking the 'master' end we hold for the corresponding
'slave', is not that important. What each of these three PTYs is
used for is much more important.
If $input, $output, and $error are too terse, spell them out as
$term_input, $term_output and $term_error or something like that,
perhaps.
Thanks.
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Junio C Hamano <gitster@pobox.com> writes:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>
>> In the ongoing effort to make the Git project a more inclusive place,
>> let's try to avoid names like "master" where possible.
>
> The two sides of a PTY are called 'master' and 'slave', and I
> understand there is a push to move away from these words, but
> calling one side with an invented name that is used by nobody else
> in the context of talking about PTY, while keeping the word used to
> call the other side, would lead to confusion.
>
> A better change is to drop "master_" altogether without replacing
> the word with anything, and call them just "input", "output" and
> "error".
If we really want to use a replacement word for 'master' instead of
just dropping, I may be inclined to suggest 'parent', in the hope
that PTY implementors will start following what Python folks are
doing, at which time they will give us a synonym for 'slave' method
called 'child' we can use.
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Junio,
On Wed, 16 Sep 2020, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> > writes:
> >
> >> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >>
> >> In the ongoing effort to make the Git project a more inclusive place,
> >> let's try to avoid names like "master" where possible.
> >
> > The two sides of a PTY are called 'master' and 'slave', and I
> > understand there is a push to move away from these words, but
> > calling one side with an invented name that is used by nobody else
> > in the context of talking about PTY, while keeping the word used to
> > call the other side, would lead to confusion.
> >
> > A better change is to drop "master_" altogether without replacing
> > the word with anything, and call them just "input", "output" and
> > "error".
>
> If we really want to use a replacement word for 'master' instead of
> just dropping, I may be inclined to suggest 'parent', in the hope
> that PTY implementors will start following what Python folks are
> doing, at which time they will give us a synonym for 'slave' method
> called 'child' we can use.
Good idea!
Thanks,
Dscho
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi Junio,
>
> On Wed, 16 Sep 2020, Junio C Hamano wrote:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>> > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
>> > writes:
>> >
>> >> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>> >>
>> >> In the ongoing effort to make the Git project a more inclusive place,
>> >> let's try to avoid names like "master" where possible.
>> >
>> > The two sides of a PTY are called 'master' and 'slave', and I
>> > understand there is a push to move away from these words, but
>> > calling one side with an invented name that is used by nobody else
>> > in the context of talking about PTY, while keeping the word used to
>> > call the other side, would lead to confusion.
>> >
>> > A better change is to drop "master_" altogether without replacing
>> > the word with anything, and call them just "input", "output" and
>> > "error".
>>
>> If we really want to use a replacement word for 'master' instead of
>> just dropping, I may be inclined to suggest 'parent', in the hope
>> that PTY implementors will start following what Python folks are
>> doing, at which time they will give us a synonym for 'slave' method
>> called 'child' we can use.
>
> Good idea!
It is unclear which one of the two you thought a good idea.
Hopefully both are good, but simply dropping "master_" without
attempting to replace would probably be better.
| @@ -1026,7 +1026,7 @@ static void handle_tags_and_duplicates(struct string_list *extras) | |||
| /* | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> In an ongoing effort to avoid non-inclusive language, let's avoid using
> the branch name "master" in a code comment.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> builtin/fast-export.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 1b8fca3ee0..5527135ba8 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -1026,7 +1026,7 @@ static void handle_tags_and_duplicates(struct string_list *extras)
> /*
> * Getting here means we have a commit which
> * was excluded by a negative refspec (e.g.
> - * fast-export ^master master). If we are
> + * fast-export ^HEAD HEAD). If we are
Obviously good change. Thanks.
> * referencing excluded commits, set the ref
> * to the exact commit. Otherwise, the user
> * wants the branch exported but every commit
| @@ -234,10 +234,10 @@ test_expect_success 'git branch -M master master should work when master is chec | |||
| git branch -M master master | |||
There was a problem hiding this comment.
On the Git mailing list, Jeff King wrote (reply to this):
On Wed, Sep 16, 2020 at 06:10:51PM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> To avoid branch names with a loaded history, we already started to avoid
> using the name "master" in a couple instances.
>
> The `t3200-branch.sh` script uses variations of this name for branches
> other than the default one. So let's change those names, as
> "lowest-hanging fruits" in the effort to use more inclusive naming
> throughout Git's source code.
A few of these are kind of odd after only this patch. E.g.:
> -test_expect_success 'git branch -M master2 master2 should work when master is checked out' '
> +test_expect_success 'git branch -M main2 main2 should work when master is checked out' '
> git checkout master &&
> - git branch master2 &&
> - git branch -M master2 master2
> + git branch main2 &&
> + git branch -M main2 main2
> '
The point of "master2" is that it wasn't "master". But now "main2" is
kind of a weird name, because it has a "2" but isn't related to
anything. If we eventually move the base branch name to "main", they'll
line up again.
I'm on the fence on whether this matters. It's a temporary
inconsistency, assuming we eventually move to "main" as the default. We
_could_ push this change off to that patch, too, but it does make it
more noisy.
But it may be that the connection to "master" here is not all that
important in the first place. And so perhaps an even better patch (both
at this stage and in the long run) is to give it a more descriptive
name. If all of these could just be "branch2", "branch3", etc, then that
alone would be better than "master2", IMHO. I'm not sure if the shared
"ma" prefix matters, though (I know in some tests it does because we're
testing glob matching).
Again, I'm on the fence whether this is exploring too deeply. It's an
opportunity to improve the tests while we're changing them. But at the
same time, I doubt anybody cares too much overall, so it feels a bit
like make-work.
-Peff
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Jeff King <peff@peff.net> writes:
> I'm on the fence on whether this matters. It's a temporary
> inconsistency, assuming we eventually move to "main" as the default. We
> _could_ push this change off to that patch, too, but it does make it
> more noisy.
Another way to handle this is perhaps to teach test-lib.sh a way to
tell it that we want to live in the world where the initial default
branch name is 'main' and use that at the beginning of these select
test scripts like t3200. Then we can do three related things in a
single patch to t3200, which are:
- Declare that any "git init" in this test (including the initial
one) uses 'main' as the default branch name;
- rename 'master' used in the test to 'main'
- rename 'master2' used in the test to 'main2'
and it would eliminate the awkwardness.
The change to test-lib.sh would likely to use init.defaultBranch
which also would be a good thing.
There was a problem hiding this comment.
On the Git mailing list, Jeff King wrote (reply to this):
On Wed, Sep 16, 2020 at 03:28:21PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I'm on the fence on whether this matters. It's a temporary
> > inconsistency, assuming we eventually move to "main" as the default. We
> > _could_ push this change off to that patch, too, but it does make it
> > more noisy.
>
> Another way to handle this is perhaps to teach test-lib.sh a way to
> tell it that we want to live in the world where the initial default
> branch name is 'main' and use that at the beginning of these select
> test scripts like t3200. Then we can do three related things in a
> single patch to t3200, which are:
>
> - Declare that any "git init" in this test (including the initial
> one) uses 'main' as the default branch name;
>
> - rename 'master' used in the test to 'main'
>
> - rename 'master2' used in the test to 'main2'
>
> and it would eliminate the awkwardness.
>
> The change to test-lib.sh would likely to use init.defaultBranch
> which also would be a good thing.
Yeah, I'd be perfectly happy with that.
-Peff
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Junio & Peff,
On Wed, 16 Sep 2020, Jeff King wrote:
> On Wed, Sep 16, 2020 at 03:28:21PM -0700, Junio C Hamano wrote:
>
> > Jeff King <peff@peff.net> writes:
> >
> > > I'm on the fence on whether this matters. It's a temporary
> > > inconsistency, assuming we eventually move to "main" as the default.
> > > We _could_ push this change off to that patch, too, but it does make
> > > it more noisy.
> >
> > Another way to handle this is perhaps to teach test-lib.sh a way to
> > tell it that we want to live in the world where the initial default
> > branch name is 'main' and use that at the beginning of these select
> > test scripts like t3200. Then we can do three related things in a
> > single patch to t3200, which are:
> >
> > - Declare that any "git init" in this test (including the initial
> > one) uses 'main' as the default branch name;
> >
> > - rename 'master' used in the test to 'main'
> >
> > - rename 'master2' used in the test to 'main2'
> >
> > and it would eliminate the awkwardness.
> >
> > The change to test-lib.sh would likely to use init.defaultBranch
> > which also would be a good thing.
>
> Yeah, I'd be perfectly happy with that.
I do want to introduce something like that in the patch series after the
next one.
However, in this instance, I think it makes more sense to use a separate
name altogether. I settled for using `topic` instead of `main2`, and
`new-topic` instead of `main3` locally.
Ciao,
Dscho
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> > Jeff King <peff@peff.net> writes:
>> >
>> > > I'm on the fence on whether this matters. It's a temporary
>> > > inconsistency, assuming we eventually move to "main" as the default.
>> > > We _could_ push this change off to that patch, too, but it does make
>> > > it more noisy.
>> > ...
> However, in this instance, I think it makes more sense to use a separate
> name altogether. I settled for using `topic` instead of `main2`, and
> `new-topic` instead of `main3` locally.
I think that is sensible. Configuration does not have to be used as
an escape hatch to make 'main2' less awkward---if we can avoid
'main2' (or 'master2'), that would be sufficient.
|
User |
In the ongoing effort to make the Git project a more inclusive place, let's try to avoid names like "master" where possible. In this instance, the use of the term `slave` is unfortunately enshrined in IO::Pty's API. We simply cannot avoid using that word here. But at least we can get rid of the usage of the word `master` and hope that IO::Pty will be eventually adjusted, too. Guessing that IO::Pty might follow Python's lead, we replace the name `master` by `parent` (hoping that IO::Pty will adopt the parent/child nomenclature, too). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
To avoid branch names with a loaded history, we already started to avoid using the name "master" in a couple instances. The `t3200-branch.sh` script uses variations of this name for branches other than the default one. So let's change those names, as "lowest-hanging fruits" in the effort to use more inclusive naming throughout Git's source code. While at it, make those branch names independent from the default branch name. In this particular instance, this rename requires a couple of non-trivial adjustments, as the aligned output depends on the maximum length of the displayed branches (which we now changed), and also on the alphabetical order (which we now changed, too). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
User |
dscho
force-pushed
the
avoid-unnecessary-branch-name
branch
from
d135a84 to
c2c1238
Compare
|
/submit |
|
Submitted as pull.734.v2.git.1600725687.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Jeff King wrote (reply to this): |
| @@ -15,24 +15,24 @@ commit_message() { | |||
| # this test script tries to document them. First, the following commit history | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
> index 79e43a370b..ba27e9d603 100755
> --- a/t/t3427-rebase-subtree.sh
> +++ b/t/t3427-rebase-subtree.sh
> @@ -15,12 +15,12 @@ commit_message() {
> # this test script tries to document them. First, the following commit history
> # is generated (the onelines are shown, time flows from left to right):
> #
> -# master1 - master2 - master3
> +# main1 - main2 - main3
The improvement between v1 and v2 made to t3200 (the previous step)
would equally apply here, no? IOW, master1 didn't have to have a
name derived from master but could just have been called topic-1.
So replacing master[0-9] with topoic[0-9] may be more appropriate
here.
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Junio,
On Mon, 21 Sep 2020, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > diff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh
> > index 79e43a370b..ba27e9d603 100755
> > --- a/t/t3427-rebase-subtree.sh
> > +++ b/t/t3427-rebase-subtree.sh
> > @@ -15,12 +15,12 @@ commit_message() {
> > # this test script tries to document them. First, the following commit history
> > # is generated (the onelines are shown, time flows from left to right):
> > #
> > -# master1 - master2 - master3
> > +# main1 - main2 - main3
>
> The improvement between v1 and v2 made to t3200 (the previous step)
> would equally apply here, no? IOW, master1 didn't have to have a
> name derived from master but could just have been called topic-1.
> So replacing master[0-9] with topoic[0-9] may be more appropriate
> here.
Indeed. It also has the benefit that `topic-` has exactly the same amount
of letters as `master`, therefore we need not worry about any ASCII art
that is modified (and might need realigning).
Thanks,
Dscho
| @@ -542,37 +542,37 @@ test_expect_success '__gitcomp - doesnt fail because of invalid variable name' ' | |||
| ' | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The completion tests used that name unnecessarily, and it is a
> non-inclusive term, so let's avoid using it here.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> t/t9902-completion.sh | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
Here, the tests and the tested feature do not work any differently
on the first-made branch, so the same "just use topic, not the
first-made branch name" used in 3/5 applies here.
Luckily, unlike 3/5 and 4/5, this step does not involve names
derived from the name of the first-made branch name, so 'main'
may be OK as-is, but for consistency across the patches, we may
want to consider using 'topic' here as well. I dunno.
There was a problem hiding this comment.
On the Git mailing list, Johannes Schindelin wrote (reply to this):
Hi Junio,
On Mon, 21 Sep 2020, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > The completion tests used that name unnecessarily, and it is a
> > non-inclusive term, so let's avoid using it here.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > t/t9902-completion.sh | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
>
> Here, the tests and the tested feature do not work any differently
> on the first-made branch, so the same "just use topic, not the
> first-made branch name" used in 3/5 applies here.
>
> Luckily, unlike 3/5 and 4/5, this step does not involve names
> derived from the name of the first-made branch name, so 'main'
> may be OK as-is, but for consistency across the patches, we may
> want to consider using 'topic' here as well. I dunno.
I am afraid that three test cases that are touched by this test rely on
the fact that the first two letters (or just the first letter) are
ambiguous when tab-completing branch names: `ma` could complete to either
`master` or `maint`.
To clarify, I added a paragraph to the commit message.
Ciao,
Dscho
|
This branch is now known as |
|
This patch series was integrated into seen via git@b4bea85. |
|
Found multiple candidates in gitster/git: Using the first one. |
|
This branch is now known as |
|
This patch series was integrated into seen via git@f335ea2. |
|
Found multiple candidates in gitster/git: Using the first one. |
|
This patch series was integrated into seen via git@c3995c6. |
|
Found multiple candidates in gitster/git: Using the first one. |
|
This patch series was integrated into seen via git@82bb329. |
|
Found multiple candidates in gitster/git: Using the first one. |
2 similar comments
|
Found multiple candidates in gitster/git: Using the first one. |
|
Found multiple candidates in gitster/git: Using the first one. |
|
This patch series was integrated into seen via git@94c094c. |
|
Found multiple candidates in gitster/git: Using the first one. |
1 similar comment
|
Found multiple candidates in gitster/git: Using the first one. |
|
This patch series was integrated into seen via git@03fa120. |
|
This patch series was integrated into seen via git@ce98771. |
The term `master` has a loaded history that serves as a constant reminder of racial injustice. The Git project has no desire to perpetuate this and already started avoiding it. The test suite uses variations of this name for branches other than the default one. Apart from t3200, where we just addressed this in the previous commit, those instances can be renamed in an automated manner because they do not require any changes outside of the test script, so let's do that. Seeing as the touched branches have very little (if anything) to do with the default branch, we choose to use a completely separate naming scheme: `topic_<number>` (it cannot be `topic-<number>` because t5515 uses the `test_oid` machinery with the term, and that machinery uses shell variables internally, whose names cannot contain dashes). This trick was performed by this (GNU) sed invocation: $ sed -i 's/master\([a-z0-9]\)/topic_\1/g' t/t*.sh Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The completion tests used that name unnecessarily, and it is a non-inclusive term, so let's avoid using it here. Since three of the touched test cases make use of the fact that two of the branch names (`master` and `maint`) start with the same letter (or even with the same two letters), we choose to replace the use of `master` by a name that also has that property: `main`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
force-pushed
the
avoid-unnecessary-branch-name
branch
from
c2c1238 to
3528588
Compare
|
On the Git mailing list, Johannes Schindelin wrote (reply to this): |
dscho
force-pushed
the
avoid-unnecessary-branch-name
branch
from
3528588 to
a80297f
Compare
|
/submit |
|
Submitted as pull.734.v3.git.1601154262.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
This patch series was integrated into seen via git@77b58c2. |
|
On the Git mailing list, Jeff King wrote (reply to this): |
|
This patch series was integrated into seen via git@0624e5d. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
|
On the Git mailing list, Jeff King wrote (reply to this): |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
|
This patch series was integrated into seen via git@635145f. |
|
This patch series was integrated into next via git@3fb4047. |
|
This patch series was integrated into seen via git@5d90f7e. |
|
This patch series was integrated into seen via git@d1936df. |
|
This patch series was integrated into seen via git@58138d3. |
|
This patch series was integrated into next via git@58138d3. |
|
This patch series was integrated into master via git@58138d3. |
|
Closed via 58138d3. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch series represents the logical next step on the journey begun with introducing
init.defaultBranch: in these patches, we avoid a couple unnecessary mentions of the branch name "master".This patch series does not try to change the default branch name, although I have that patch series ready to go. You can see the overall idea here: #655. Concretely, I plan on submitting three more patch series after this one:
mainin the test suite. This is necessary because my plan is to change the default branch name to that name, therefore it cannot be used as the name of a topic branch any longer.main. Most of the patches provide non-trivial (read: non-scriptable) adjustments to the test suite in an incremental fashion, with a big patch toward the end that reflects a fully-automated search-and-replace of all the trivial cases.(Note: I am still debating whether I should move one or two patches from the second to the third patch series)
Changes since v2:
topicinstead ofmainto patch 4/5.topicinstead ofmainhere.Changes since v1:
primaryfor the adjustments tot/test-terminal.perl, we follow Python's lead (which uses the parent/child nomenclature).mainas branch names; Instead, the renamed branches are independent from any current or future default branch name.cc: Jeff King peff@peff.net
cc: Johannes Schindelin Johannes.Schindelin@gmx.de