Skip to content

Contributing guidelines#422

Merged
ariporad merged 3 commits intomasterfrom
contributing-guidelines
Apr 14, 2016
Merged

Contributing guidelines#422
ariporad merged 3 commits intomasterfrom
contributing-guidelines

Conversation

@nfischer
Copy link
Copy Markdown
Member

Opening this as a pull request, since I think it's time we start seriously discussing this.

I modified the contributing guidelines @ariporad wrote up a while ago. This has been updated due to some changes in Github, such as the "Update branch" button and the squash-merge feature. There's no need to bother to tell people to squash commits if Github is going to do it anyway for us.

The idea behind these guidelines is to be guidelines, not strict rules. If someone sees the link and wants to follow the contributing guidelines, that's awesome. If not, that's ok too. I don't want to be overly strict and deter contributors in the process.

I also added a Github issue template (for bug reports). The majority of issues we get are bug reports, this just helps us get the info we need sooner rather than later (OS, node version, ShellJS version, etc.). I figure if someone wants to do a feature request, they can figure out that "OS" and "ShellJS version" don't really apply, and they'll just format their Github issue in a way that makes sense.

@shelljs/contributors please give input, since I want to make sure these guidelines aren't going to deter contributions.

@nfischer nfischer added the chore label Apr 11, 2016
@nfischer
Copy link
Copy Markdown
Member Author

This is based off the instructions here

reported it, please ping that issue thread.
- Let us know your version of NodeJS (`node -v`), your version of ShellJS (from
`package.json`), your OS, and any other useful information.
- Give an example ShellJS command to reproduce the error.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know how we would say this, but we get a lot of issues that say "This script hangs/has no output/exits X", but then they can't or wont tell us what the script actually is, and it would be nice to cut down on that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I said "Example ShellJS command to reproduce the error:" Pretty sure that'll do the trick. Let me know if you think otherwise.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I phrased that poorly.

A lot of people will give us something like this:

exec('command', function(...) {
    // Something is broken here
});

But they wont or can't tell us what command is, so we can't debug it. So maybe add something like Please include the full commands of any exec's you use

@nfischer nfischer force-pushed the contributing-guidelines branch from d205e21 to 179b32b Compare April 14, 2016 03:50
@nfischer
Copy link
Copy Markdown
Member Author

@ariporad comments have been addressed

CONTRIBUTING.md Outdated

- Please add tests for all changes/new features.
- Make sure your code passes `npm test`. Please check the CI (both Appveyor and
Travis) to make sure it passes). If you can't figure out why something
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's an extra paren here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

@nfischer nfischer force-pushed the contributing-guidelines branch from 179b32b to ef0678f Compare April 14, 2016 04:38
@ariporad
Copy link
Copy Markdown
Contributor

LGTM!

@ariporad ariporad merged commit 0c85cc1 into master Apr 14, 2016
@ariporad ariporad deleted the contributing-guidelines branch April 17, 2016 14:55
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.

2 participants