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

Overhaul CONTRIBUTING document #1103

Merged
merged 3 commits into from
Nov 11, 2023

Conversation

hyperupcall
Copy link
Collaborator

Inspired by this comment, this updates the CONTRIBUTING.md and moves the information about testing dependencies to itself.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@hyperupcall hyperupcall force-pushed the hyperupcall-contributing-docs branch from b4a5cab to 6e6e9bc Compare November 6, 2023 11:01
CONTRIBUTING.md Outdated

Let's assume your new command is named `foo`.
- macOS, Linux, BSDs (You may need to browse their man page)
- Bash 3.2+ (If you aren't sure, see the [Bash changelog](https://git.savannah.gnu.org/cgit/bash.git/tree/NEWS?h=devel) to see a detailed list of version support for features)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for updating the stale doc. Now we need Bash 4+, and there is other new dependencies: https://github.com/tj/git-extras/blob/main/Installation.md#dependencies

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, I missed Installation.md, I'll reference that instead of duplicating the same information in CONTRIBUTING

CONTRIBUTING.md Outdated
Let's assume your new command is named `foo`.
- macOS, Linux, BSDs (You may need to browse their man page)
- Bash 3.2+ (If you aren't sure, see the [Bash changelog](https://git.savannah.gnu.org/cgit/bash.git/tree/NEWS?h=devel) to see a detailed list of version support for features)
- Git 2.1+
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it is time to update the git version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll update it to Git 2.17.1, which is a less ancient Git version and is the lowest common denominator between Ubuntu 18 and Debian 10. I'm not sure if it is a good idea to increase it further than that, because I don't know which versions we truly support? Would we have to check all our uses of git to find out, or do you have an idea yourself?

Copy link
Collaborator

Choose a reason for hiding this comment

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

2.17 LGTM

@spacewander
Copy link
Collaborator

Please make the CI pass, thanks!

@hyperupcall
Copy link
Collaborator Author

@spacewander It's fixed now - sorry about that

@spacewander
Copy link
Collaborator

@spacewander It's fixed now - sorry about that

No need to be sorry for it.

@spacewander spacewander merged commit 7a65311 into tj:main Nov 11, 2023
5 checks passed
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