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

Add Contributing guideline #555

Merged
merged 3 commits into from
Jul 31, 2016
Merged

Conversation

spacewander
Copy link
Collaborator

@hemanth @qw3rtman @nicolaiskogheim
Let's have a look at this fresh guideline!

@nicolaiskogheim
Copy link
Collaborator

Could you split this PR in 3? :)
One for contributing guidelines, one for check_integrity.sh, and one for the rest.

@spacewander
Copy link
Collaborator Author

Done

@@ -0,0 +1,16 @@
## Your new git-extra command should support...
Copy link
Collaborator

Choose a reason for hiding this comment

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

## Your new git-extra command should support

@nicolaiskogheim
Copy link
Collaborator

Thanks. Great work! I have added my two cents.

@nicolaiskogheim
Copy link
Collaborator

At the bottom we could add something like:

You are welcome to open up an issue to discuss new commands or features before opening a pull request.

## Your new git-extra command should support...

* Should support Mac and Linux(You may need to browse their man page)
* Should support Bash 3 or newer(If you are not sure, [Bash versions](http://tldp.org/LDP/abs/html/bash2.html))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't actually think we will have much difficulties supporting Bash 3.0 (at least that's the impression I got from scanning over the changelogs), but it may be difficult for people to test it. I tried building Bash 3.0 on OSX, and I was unable to, even with all the patches.

Now, the default shell on my OSX El Capitan is 3.2 so that one shouldn't be hard to get hold of.

Should we say Bash >= 3.2?

@spacewander
Copy link
Collaborator Author

What about the latest version of CONTRIBUTING.md?

@@ -0,0 +1,18 @@
## Your new git-extra command should support

* OSX, Ubuntu, FreeBsd(You may need to browse their man page)
Copy link
Collaborator

@qw3rtman qw3rtman Jul 29, 2016

Choose a reason for hiding this comment

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

Perhaps OS X, Ubuntu, and FreeBSD (you may need to check the documentation for compatibility issues)*?

We could also add a note at the bottom (hence the *) saying something along the lines of "if you aren't able to test your new command on a platform, make that clear in your PR and someone else may be able to test it on their system"

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm in favor of both of those suggestions.

@qw3rtman
Copy link
Collaborator

qw3rtman commented Jul 29, 2016

LGTM, @spacewander! I just mainly made a couple formatting changes along with one suggestion regarding what to do if you are unable to test the command on a particular OS.

On the topic of which OS's to support, should we explicitly include CentOS/RHEL as a requirement? Or would that be covered by Ubuntu already?

@spacewander
Copy link
Collaborator Author

@qw3rtman
I guess more people use ArchLinux as their desktop distribution than CentOS/RHEL...
Let's use Linux to simply represent all the distributions.

@qw3rtman
Copy link
Collaborator

@spacewander

I'm good with that. So let's replace Ubuntu with Linux and keep OS X and FreeBSD (or just BSD?) (since BSD != Linux)

@nicolaiskogheim, what do you think?

@nicolaiskogheim
Copy link
Collaborator

OSX, Linux, BSD is fine by me 👍

@nicolaiskogheim
Copy link
Collaborator

Let's just hold this until #557 is in.

@hemanth
Copy link
Collaborator

hemanth commented Jul 31, 2016

#557 is in, this is good to merge.

@nicolaiskogheim nicolaiskogheim merged commit f3954e1 into tj:master Jul 31, 2016
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.

4 participants