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 contribution guidelines v1 #250

Merged
merged 9 commits into from
Dec 15, 2017
Merged

Add contribution guidelines v1 #250

merged 9 commits into from
Dec 15, 2017

Conversation

wbhob
Copy link
Contributor

@wbhob wbhob commented Nov 17, 2017

This is the first attempt at writing contribution guidelines for Nest.

…s properly formatted. I\'ve included a command, [20:37:16] Using gulpfile ~/nest/gulpfile.js

[20:37:16] Starting 'format'...
[20:37:22] Finished 'format' after 5.76 s that will take care of this for you.\nThis functionality is taken directly from Angular's repository.
…commit is properly formatted. I\'ve included a command, [20:37:16] Using gulpfile ~/nest/gulpfile.js"

This reverts commit c4a3e02.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.674% when pulling 77817b2 on wbhob:contrib into cbb80b6 on nestjs:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.674% when pulling 5b92c33 on wbhob:contrib into cbb80b6 on nestjs:master.

Copy link

@MonsieurMan MonsieurMan left a comment

Choose a reason for hiding this comment

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

I think we need to quote the angular team somewhere as we used their work.

@@ -1,6 +1,6 @@
# dependencies
node_modules/

package-lock.json

Choose a reason for hiding this comment

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

Isn't the whole purpose of this file to be versioned ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason Kamil keeps removing it from the repo, so I thought I'd add it to gitignore

Choose a reason for hiding this comment

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

You shouldn't have done it in this PR if it's not related to it then. You're creating side effects, haha.
@kamilmysliwiec what's up with it ?

CONTRIBUTING.md Outdated

We have very precise rules over how our git commit messages can be formatted. This leads to **more
readable messages** that are easy to follow when looking through the **project history**. But also,
we use the git commit messages to **generate the Angular change log**.

Choose a reason for hiding this comment

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

Angular made it's way through !
By the way is there really no way we can suggest changes to a PR ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In your PR review, just set it to rejected

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

[print, sign and one of scan+email, fax or mail the form][corporate-cla]. -->


<!-- [angular-group]: https://groups.google.com/forum/#!forum/angular -->

Choose a reason for hiding this comment

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

Angular's sneaking everywhere !

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 commented it out here, since I don't really know what to do with this link for now 😄



<!-- [angular-group]: https://groups.google.com/forum/#!forum/angular -->
<!-- [coc]: https://github.com/angular/code-of-conduct/blob/master/CODE_OF_CONDUCT.md -->

Choose a reason for hiding this comment

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

Here also !
As the SO part below, waiting for further work ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Commented

[jsfiddle]: http://jsfiddle.net
[plunker]: http://plnkr.co/edit
[runnable]: http://runnable.com
<!-- [stackoverflow]: http://stackoverflow.com/questions/tagged/angular -->

Choose a reason for hiding this comment

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

And here too !
I guess you commented it out until we have further information about SO ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's correct!

- and most importantly - a use-case that fails

<!--
// TODO we need to create a playground, similar to plunkr

Choose a reason for hiding this comment

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

We need to file an issue for this one.

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'll probably hack one together

* **packaging**: used for changes that change the npm package layout in all of our packages, e.g. public path changes, package.json changes done to all packages, d.ts file/format changes, changes to bundles, etc.
* **changelog**: used for updating the release notes in CHANGELOG.md
* **examples/#**: for the example apps directory, replacing # with the example app number
<!-- * **aio**: used for docs-app (angular.io) related changes within the /aio directory of the repo -->

Choose a reason for hiding this comment

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

It's there too !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, it is commented out until we need this line

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.674% when pulling 447b8ab on wbhob:contrib into cbb80b6 on nestjs:master.

@kamilmysliwiec
Copy link
Member

Looks good 🙂 thanks @wbhob for your effort. Should I take care of the rest?

@wbhob
Copy link
Contributor Author

wbhob commented Nov 20, 2017

Yeah, @kamilmysliwiec this is more just a starting point. It's literally a copy/paste from Angular's contribution guidelines. Feel free to adjust where you see fit.

@wbhob
Copy link
Contributor Author

wbhob commented Nov 25, 2017

@kamilmysliwiec what do I need to do to get this merged?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.746% when pulling 82cb61a on wbhob:contrib into 832eb51 on nestjs:master.

@wbhob
Copy link
Contributor Author

wbhob commented Dec 12, 2017

Do you have an update @kamilmysliwiec ?

@kamilmysliwiec
Copy link
Member

🎉

@kamilmysliwiec
Copy link
Member

That's good for now. Thanks @wbhob :)

@kamilmysliwiec kamilmysliwiec merged commit 4d4f7a4 into nestjs:master Dec 15, 2017
@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.708% when pulling ade7aed on wbhob:contrib into 3e0ee21 on nestjs:master.

@lock
Copy link

lock bot commented Sep 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants