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

Simplify and deduplicate installation instructions #5260

Merged
merged 4 commits into from Feb 4, 2023

Conversation

steffen
Copy link
Contributor

@steffen steffen commented Jan 20, 2023

Today I wanted to install Git LFS on a Linux-based system.
Unfortunately I had a hard time to find the complete installation instructions. 😩

Reproduction steps

  1. I first opened git-lfs.com on my macOS machine and I did not see any link to installation instructions for Linux: image
  2. I then went to GitHub's documentation for installing Git LFS which besides pointing to git-lfs.com (where I was already unsuccessful) it also points to the README's Getting Started section.
    I was happy to find a Linux users mention with a link to installation instructions on packagecloud.
  3. I performed the "quick install" command: curl -s https://packagecloud.io/install/repositories/github/git-lfs/script.deb.sh | sudo bash
    Unfortunately Git LFS was still not installed (I got git: 'lfs' is not a git command. See 'git --help'. when running git lfs). 😢
  4. I then looked at what the "quick install" script does and noticed that it only adds the packagecloud repository to apt, but it does not install it yet.
    I then ran apt-get install git-lfs for which I got a permission error.
  5. I then ran sudo apt-get install git-lfs which finally installed Git LFS on my Linux system. 😅

Baffled by the unclear installation instructions I then found that there is a whole document dedicated to installing Git LFS on Linux:
https://github.com/git-lfs/git-lfs/blob/main/INSTALLING.md

This document is currently not linked to from the README (or the website at git-lfs.com).

Proposal ☺️

I propose to simplify and deduplicate the README's installation instructions as well as adding the missing link to INSTALLING.md.

First I noticed that there is a Downloading section which starts with a "You can install ..." sentence and that there is a Installing section.
Both sections include information on installing from binary and from source, which seem to be redundant?

I therefore propose to consolidate the Downloading and Installing sections into one Installing section.
This is the main change proposal of this pull request besides adding a link to INSTALLING.md.

I also propose to make the INSTALLING.md document a little bit more clear.
I removed some redundant steps information in the beginning of the document and I added "1." and "2." numbering to the sections to make it clear there are two steps.
(I also lowercased some confusing uppercase mentions of apt and yum. 😊)

I may also open have opened a pull request for https://github.com/git-lfs/git-lfs.github.com to make these README installation instructions easier to discover: git-lfs/git-lfs.github.com#55

Copy link
Contributor

@chrisd8088 chrisd8088 left a comment

Choose a reason for hiding this comment

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

Hey, thanks for the patch, this looks like a good improvement overall! I had a couple of ideas and suggestions, but nothing major. Thanks again and let me know what you think of my comments!

README.md Outdated Show resolved Hide resolved
README.md Outdated
Comment on lines 55 to 60
- Ensure you have the latest version of Go, GNU make, and a standard Unix-compatible build environment installed.
- On Windows, install `goversioninfo` with `go install github.com/josephspurrier/goversioninfo/cmd/goversioninfo@latest`.
- Run `make`.
- Place the `git-lfs` binary, which can be found in `bin`, on your system’s executable `$PATH` or equivalent.
- Git LFS requires global configuration changes once per-machine. This can be done by
running:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'd like to keep these instructions on building from source, as they were specifically updated in PR #4527 to capture various details which aren't in the Wiki page, and which we'd like to have right up front for users. Would you mind restoring these in the From source section? (We should probably drop the unnecessary hyphen in "per-machine", though.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @chrisd8088 Thank you for your review and feedback! 🙇‍♂️ ❤️

I found the wiki with the Installation instructions only at the end of performing my pull request changes (since it was only linked to from the "From source" section).

Now I have to ask, why are there installation instructions in the README and in the wiki? 😊
From the user perspective I find it quite confusing as you don't know which is the most up-to-date and the source of truth, from the maintainer perspective I also would find it as an unnecessary overhead?

Wouldn't it make sense to consolidate these instructions in one place? 🤔
(I'm happy to help with that.)

The overall goal would be to make it as easy and straight forward for users to install Git LFS and reduce confusion and friction.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would assume that different folks created the two sets of instructions at different times. I think it's probably advisable for the Wiki to refer to the canonical instructions in the repo's README.md rather than the other way around.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's probably advisable for the Wiki to refer to the canonical instructions in the repo's README.md rather than the other way around.

I agree! We can then link to the README from the wiki.

If it's 👍 for you I would work then on moving and consolidating the installation instructions in the README and update this PR accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I was only talking about the Wiki's copy of these particular notes, not the whole INSTALLING.md page, if that was what you were thinking. If you don't mind restoring these notes in the README.md, that would get my approval -- thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you don't mind restoring these notes in the README.md, that would get my approval -- thank you!

Hi @chrisd8088,

In 6b2aa30 I have restored the "From source" section from the original README.md.
The only change I made is making git lfs install inline as suggested in #5260 (comment).

For convenience here is the updated rendered README.md:
https://github.com/steffen/git-lfs/blob/main/README.md

I think these changes should make the PR approvable.
As discussed, further potential improvements I will propose in separate bitesized pull requests over time.

Thank you again for your review and feedback! 🙇‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks great, thank you!

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@chrisd8088 chrisd8088 left a comment

Choose a reason for hiding this comment

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

Thanks very much for this cleanup!

@chrisd8088 chrisd8088 merged commit c66fd28 into git-lfs:main Feb 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants