Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CONTRIBUTING.md: 💅 #3325

Merged
merged 4 commits into from
Oct 17, 2018
Merged

CONTRIBUTING.md: 💅 #3325

merged 4 commits into from
Oct 17, 2018

Conversation

ttaylorr
Copy link
Contributor

This pull request compiles a few miscellaneous changes that I've been sitting on for the CONTRIBUTING.md document, that I had mentioned (and was reminded of) in #3317 (review).

/cc @git-lfs/core

We use the Markdown trick that the same number over and over again
produces a list of numbers incrementing starting at that base.

This is good, since it reduces the diff later on should the list
ordering change, grow new elements, etc. But it is not desirable to
start the list at zero.

Let's instead increment the list starting at 1, that way we get lists of
the form (1), (2), (3), instead of (0), (1), (2).
We no longer use the GitHub issue label `core-team`, hence let's remove
it to indicate it as such.
Since the advent of [1], and [2], we use Go 1.11 with modules enabled.
This relieves us of the requirement of having Git LFS checked out within
the appropriate location beneath the caller's $GOPATH.

So, let's remove the guideline, and recommend away from using `go get`,
since this command behaves differently based on whether or not its CWD
is itself a Go repository.

Instead, recommend that we use `git clone`, and do not specify a
location to perform the clone in, since Git LFS can be checked out from
anywhere.

[1]: dfc0f2f (Merge pull request #3298 from git-lfs/ttaylorr/go-1.11.1,
     2018-10-08)
[2]: 35fe301 (Merge pull request #3208 from git-lfs/ttaylorr/go-mod,
     2018-08-29)
ShellSession gives us marginally better highlighting, comparing

    $ foo
    $ bar

with:

    ```ShellSession
    $ foo
    $ bar
    ```

So, where appropriate, let's convert the former into the later.
@ttaylorr ttaylorr added this to the v2.6.0 milestone Oct 17, 2018
@ttaylorr ttaylorr requested review from bk2204 and a team October 17, 2018 18:00
Copy link
Member

@bk2204 bk2204 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Thanks for cleaning this up. I had expected an issue with things still to be tidied, and instead you sent in a pull request, saving me the work.

0. Make your change, add tests, and make sure the tests still pass
0. Push to your fork and [submit a pull request][pr] from your branch to `master`
0. Pat yourself on the back and wait for your pull request to be reviewed
1. [Fork][] and clone the repository
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I'm personally partial to zero-based indexing, I agree it's not intuitive to most people, and one-based numbering is in agreement with typical style guides, so I think this is a good way forward.

Copy link
Contributor

@PastelMobileSuit PastelMobileSuit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cared-for nails

@ttaylorr ttaylorr merged commit 6a05989 into master Oct 17, 2018
@ttaylorr ttaylorr deleted the ttaylorr/contributing-misc-tweaks branch October 17, 2018 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants