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

Fix Issue #15, created Contributing.md #16

Merged
merged 7 commits into from
Nov 10, 2018
Merged

Fix Issue #15, created Contributing.md #16

merged 7 commits into from
Nov 10, 2018

Conversation

CometS1
Copy link
Contributor

@CometS1 CometS1 commented Nov 10, 2018

Created Contributing.md and added into the README basic instructions to run the app.

Copy link
Owner

@Mera-Gangapersaud Mera-Gangapersaud left a comment

Choose a reason for hiding this comment

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

Great work @CometS1

Only suggestion is adding a link to Contributing.md at the end of the README.md

@CometS1
Copy link
Contributor Author

CometS1 commented Nov 10, 2018

Updated Readme for contributing link

Copy link
Contributor

@seanprashad seanprashad left a comment

Choose a reason for hiding this comment

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

Thank you for your first contribution @CometS1! 🎊

CONTRIBUTING.md helps new individuals get up and running more easily, as well as find help if they need it. We're excited to land this 🙉

We love what you've done and have a few suggestions to make this even better. Please don't hesitate to let us know if you have any questions!

CONTRIBUTING.md Outdated
### Running the app

```
$ npm install
Copy link
Contributor

Choose a reason for hiding this comment

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

We should consider the vast range of individuals who might want to contribute - some of them might not be familiar with what each command does 🤔

Could we add a brief comment beside each command like so:

$ npm install    # install required packages for creative collab
$ npm start      # start creative collab at localhost:3000

CONTRIBUTING.md Outdated
Please use the [Issue Tracker](https://github.com/Mera-Gangapersaud/Creative-Collab/issues) to discuss
potential improvements you want to make before sending a Pull Request.

The Issue Tracker may contain items labelled **help wanted** or **good first issue**
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ this! We actually use the label good first bug 🐛 instead of good first issue - would you be able to swap those out?

It would also be sweet to link individuals directly to those issues, like so:

[**good first bug**](https://github.com/Mera-Gangapersaud/Creative-Collab/labels/good%20first%20bug%20%F0%9F%90%9B)
[**help wanted**](https://github.com/Mera-Gangapersaud/Creative-Collab/labels/help%20wanted%20%F0%9F%99%8F%F0%9F%8F%BC)


## Pull Requests

[Pull Requests](https://help.github.com/articles/about-pull-requests/) are used for contributions. Code must be well-tested and not decrease the test coverage significantly.
Copy link
Contributor

Choose a reason for hiding this comment

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

Spot on 💯

CONTRIBUTING.md Outdated
Please commit changes to a separate branch in your fork
so that we can work together making changes to it before it
is ready to be merged. Name your branch something like
`<username>/feature`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you very much for this! It might be better to have contributors name their branches after the issues they're fixing, like so:

username/issue-X where X is the issue number their pull request addresses

README.md Outdated
```

### Contributing
Please see the Contributing [guidelines](CONTRIBUTING.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Let's add a period like so:

Please see the Contributing [guidelines](CONTRIBUTING.md).

README.md Outdated
### Running the app
You must have these or a later version installed:

Node version 10.10 | npm version 6.4.1
Copy link
Contributor

@seanprashad seanprashad Nov 10, 2018

Choose a reason for hiding this comment

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

Nice - just to make it easier to read, could we update this to a bulleted list, like so:

  • Node.js version 10.10
  • npm version 6.4.1

To get that look, we'd need to do something like:

* Node.js version 10.10
* npm version 6.4.1

Made minor changes to Contributing doc
@seanprashad seanprashad self-requested a review November 10, 2018 18:47
Made minor changes to Readme
@CometS1
Copy link
Contributor Author

CometS1 commented Nov 10, 2018

Updated docs for minor changes.

Copy link
Contributor

@seanprashad seanprashad left a comment

Choose a reason for hiding this comment

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

I overlooked one minor thing that will affect how new contributors name their branches.

After we've fixed that up, this will be good to 🚢 !!

CONTRIBUTING.md Outdated

Please commit changes to a separate branch in your fork
so that we can work together making changes to it before it
is ready to be merged. Name your branch something like
Copy link
Contributor

@seanprashad seanprashad Nov 10, 2018

Choose a reason for hiding this comment

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

Oversight on my part - when an individual forks Creative Collab, it won't be Mera-Gangapersaud/Creative-Collab but Username/Creative-Collab.

With the current instructions, users will be creating their branches like so:

  • git checkout -b username/issue-999

instead of:

  • git checkout -b issue-999

Could we update this to:

Branch names should follow the format of issue-X, where X is the issue number that your pull request fixes.

@CometS1
Copy link
Contributor Author

CometS1 commented Nov 10, 2018

Updated for branch names

@seanprashad seanprashad self-requested a review November 10, 2018 19:13
Copy link
Contributor

@seanprashad seanprashad left a comment

Choose a reason for hiding this comment

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

Thank you for the quick turn-around @CometS1! We look forward to seeing you help develop Creative Collab into something spectacular 🥇✨

@seanprashad seanprashad merged commit 3ad0ca3 into Mera-Gangapersaud:master Nov 10, 2018
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.

3 participants