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

FIXES #1224 - Added contribution section to README.md #1225

Merged
merged 8 commits into from Feb 7, 2018

Conversation

consideRatio
Copy link
Contributor

@consideRatio consideRatio commented Feb 4, 2018

Fixes #1224 by providing a section in the README.md on how to setup a development environment.

@juhasch
Copy link
Member

juhasch commented Feb 4, 2018

Thank you for your contribution. This is a good idea and we were actually discussing this here: #1213.

I think it is better adding a new CONTRIBUTING.md and just add a link in the README.md.
This way it is easier to add new content without making the readme unwieldily large.

Can you change the PR to provide a CONTRIBUTING.md ?
You can also take a look at the Wiki entry here:
https://github.com/ipython-contrib/jupyter_contrib_nbextensions/wiki/Create-a-new-Notebook-Extension

@consideRatio
Copy link
Contributor Author

Thank you for the input @juhasch ! This is now based this on @benelot wiki entry. I added a section on how to setup development and tried to update the text to an audience interested in contributing in some way but not necessarily creating an extension.


Contributing to jupyter notebook nbextensions

We are super happy that you intend to contribute to the nbextensions! You can discuss improvements in issues and improvement them in pull requests.

Create an issue

Do not hesitate to open up an issue, you can discuss bugs, improvements or new extensions in them. Creating an issue is a good starting point for code contributions. The community can support you with experience of similar extensions, pros and cons, what to look for etc.

Here is an example issue of how @benelot did it that worked pretty smoothly: #1193

Setup development

  1. Fork and clone
    First fork this repo, then clone your fork. In this way you become prepared to make a pull request.
# clone your fork
git clone https://github.com/<your-github-username>/jupyter_contrib_nbextensions.git
cd jupyter_contrib_nbextensions
  1. Setup
# run from the main directory, where setup.py is
pip install --editable .
jupyter-contrib-nbextension install --symlink --sys-prefix

Create an extension

Add a folder with the name of your new extension to jupyter_contrib_nbextensions/nbextensions. Check out the Jupyter Notebook extension structure link to know what has to be in that folder and what the general conventions are.

Create a pull request

As you are ready with your code contribution, make a pull-request to the main repo and briefly explain what you have done.

Here is an example pull request of how @benelot did it that worked super well: #1213

Please also update the CHANGELOG.md.

Copy link
Member

@jcb91 jcb91 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @consideRatio for your work on this. There's a comments inline, plus could you add CONTRIBUTING.md to the MANIFEST.in? Thanks again 😄

CONTRIBUTING.md Outdated
```shell
# run from the main directory, where setup.py is
pip install --editable .
jupyter-contrib-nbextension install --symlink --sys-prefix
Copy link
Member

Choose a reason for hiding this comment

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

Although a good idea where available, --symlink doesn't work properly on Windows.
This could probably benefit from a little explanation.

CHANGELOG.md Outdated
@@ -47,6 +47,9 @@ Repo-level stuff:
[#1200](https://github.com/ipython-contrib/jupyter_contrib_nbextensions/pull/1200),
[#1201](https://github.com/ipython-contrib/jupyter_contrib_nbextensions/pull/1201)
[@jcb91](https://github.com/jcb91)
- Added CONTRIBUTION.md
Copy link
Member

Choose a reason for hiding this comment

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

This should go in the unreleased/github master section, since 0.4.0 has already been released.

Copy link
Member

Choose a reason for hiding this comment

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

Also, it's contributING.d (not contributION.md)

CONTRIBUTING.md Outdated
@@ -0,0 +1,38 @@
# Contributing to jupyter notebook nbextensions

We are super happy that you intend to contribute to the nbextensions! You can discuss improvements in issues and improvement them in pull requests.
Copy link
Member

Choose a reason for hiding this comment

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

I think this was meant to read 'and implement them in pull requests'?

@consideRatio
Copy link
Contributor Author

Thank you for the code review @jcb91 !! I'll fix it all!

@consideRatio
Copy link
Contributor Author

@jcb91 done!

Technical detail: I made a rebase on this repo's updated master branch, and then a force push to my fork, making your code review comments dangle as those commits no longer have the same hash. I can't see how it would affect anything other than that though, but my experience is lacking when it comes to this.

consideRatio and others added 3 commits February 5, 2018 00:16
* using github for small doc-edit type PRs
* conda/virtualenv activation
* which bit of changelog to update
@jcb91
Copy link
Member

jcb91 commented Feb 6, 2018

Thanks @consideRatio, rebase is fine. I've added a couple more clarification notes about virtual environments, and the possibility of using github for small direct edits of things like docs. Looks good to merge, I think

@consideRatio
Copy link
Contributor Author

Ah nice! This looks good! I did not know about Github edits!


Off topic: I was confused by how you have made edits to this PR, and have tried to learn about it by googling and reading.

I'm thinking of questions like:

  • Where is this changed PR code stored?
  • What would happen if I made additional changes now?

I read the following, but is still confused.

@jcb91
Copy link
Member

jcb91 commented Feb 6, 2018

I was confused by how you have made edits to this PR

It was perhaps a little impolite of me to just push, and it might have been nicer/less confusing to make a PR to your fork from mine. For how it works, I've attempted an outline below:

I'm thinking of questions like:

Where is this changed PR code stored?
What would happen if I made additional changes now?

Essentially what's going on here is that I have pushed commits to the readme-pr branch of your github fork consideRatio/jupyter_contrib_nbextensions. There's a small suggestion that this is possible near the bottom of the PR page, just above the status box:

Normally, I would not have permission to push to your fork, but since September 2016, GitHub PRs provide a setting which allows anyone with push access for the repo to which you're making the PR (in this case, I have push access for ipython-contrib/jupyter_contrib_nbextensions) to push to the PR branch on the fork from which the PR comes (in this case, readme-pr on your fork). You can set this when creating a PR:

Or afterwards for PRs you own (opened) - see the tiny checkbox at the bottom of the right-hand side panel:

Also, this help page gives some more info and links.

So, if you were to make further changes to your local branch, you'd end up having diverged from your github repo. To avoid this, you'd need to either pull changes from your github repo before making further edits locally, or else rebase/merge changes already made locally onto/into the newly-updated github branch, before pushing the updated history up to your fork again. Make sense?

@consideRatio
Copy link
Contributor Author

Wow @jcb91 that made perfect sense, thank you a lot for the effort of explaining this to me, I could get it very very quickly due to this thorough explanation!

@jcb91 jcb91 merged commit 02a9fb9 into ipython-contrib:master Feb 7, 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.

None yet

4 participants