-
Notifications
You must be signed in to change notification settings - Fork 370
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
updated contributors doc #1301
updated contributors doc #1301
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks good, really don't want emoji creeping in though. Left a few more specifics.
# CaImAn contributors guide | ||
CaImAn is an open source project where *everyone* is welcome and encouraged to contribute. We have external contributors from all over the world, and we are always looking for more help. The guide below explains how to contribute to Caiman: if you have questions about the process, please feel free to reach out at [Gitter](https://app.gitter.im/#/room/#agiovann_Constrained_NMF:gitter.im). Everyone typically needs some help at first, so please don't be shy about asking. | ||
# CaImAn contributors guide :hammer: | ||
CaImAn is an open source project where *everyone* is welcome and encouraged to contribute. We have external contributors from all over the world, and we are always looking for more help. This guide explains how to contribute: if you have questions about the process, please feel free to reach out at [GitHub Discussions](https://github.com/flatironinstitute/CaImAn/discussions). Everyone needs help contributing and finds git/GitHub confusing, so please don't be shy about asking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's some language in here I'd like to trim (everyone finds git/github confusing? That's a bit much) but it's fine.
I don't really want emojis to keep creeping into more of our docs though.
|
||
There are many different ways you can contribute to Caiman. The first and easiest way is to bring problems to our attention: if you find a bug, or think there is a feature that is lacking in Caiman, please [open an issue at Github](https://github.com/flatironinstitute/CaImAn/issues). You can also contribute simply by *participating* in the different forums. | ||
There are many different ways you can contribute to Caiman. The first and easiest way is to bring problems to our attention: if you find a bug, or think there is a feature that is lacking in Caiman, please [open an issue at Github](https://github.com/flatironinstitute/CaImAn/issues). You can also contribute just by *participating* in the different forums. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we don't need the word "just" at all, as we didn't "simply" ; probably better to remove it
CONTRIBUTING.md
Outdated
|
||
Second, let's say you want to improve something yourself: | ||
|
||
- Documentation like what you are currently reading | ||
- The demo notebooks | ||
- The code base | ||
|
||
We welcome all such contributions. To do this you need to make changes on your local version of Caiman then push this to the repository by making a **Pull Request** (PR). We will walk you through this process in the rest of the document. | ||
We welcome *all* such contributions. To make them, you need to make changes on your local version of Caiman and then push make a **Pull Request** (PR) to our GitHub repository. We will walk through this process in the rest of the document. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why bold all?
|
||
Before you go through the work required to improve something, we recommend that you let us know your plans. E.g., open an issue, or reach out on Gitter. This way, we can avoid duplicated effort (if someone is already working on it), or wasted time (it could turn out the changes might not be feasible right now). If needed, can usually set up a video chat on Google or Zoom to talk about a feature proposal if that works for you. | ||
Before you go through the work required to improve something, we recommend that you let us know your plans on GitHub either in discussions or issues. This way, we can avoid duplicated effort (if someone is already working on it), or wasted time (it could turn out the changes might not be feasible right now because it conflicts with some other major feature we are working on). If needed, can usually set up a video chat to talk about a feature proposal if that works for you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or if we think it's a bad idea (however we might want to phrase that). We don't want to promise we'll toss something in.
|
||
There are many possible workflows for contributing to open source projects (see a summary [here](https://docs.gitlab.com/ee/topics/gitlab_flow.html)). The basic structure of the workflow we use at Caiman is illustrated here: | ||
There are [many possible workflows](https://www.atlassian.com/continuous-delivery/continuous-integration/trunk-based-development) for contributing to open source projects. The basic structure of the workflow used at Caiman is illustrated here: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine for now, but if we can find a less wordy and less comprehensive thing to point to someday that'd be better. No rush.
|
||
where `<your-username>` is replaced by your GitHub username. You now have a local version of the repo! | ||
|
||
3. Add a remote upstream version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might or might not be necessary; the FI remote isn't strictly needed to issue a PR provided we never need to ask somebody to rebase or something
|
||
<img src="docs/img/pull_request.jpg"> | ||
|
||
When you click the `Compare & pull request` button, it will bring up an interface to integrate your code at the main Caiman repo. Be sure to generate your PR targeting the `dev` branch, not `main`, and fill out the PR template that is auto-generated by Caiman with the information requested. | ||
|
||
Note that all PRs are reviewed by other programmers. This is an important part of the process: they will almost always give comments, making suggestions or asking questions about changes using Github's pull request interface. | ||
|
||
You may be asked to make some changes (or to *think* about making some changes). You will sometimes need to do more some more work on your branch and make more changes after making an initial PR. In this case, the workflow is simple: you will work within your your local `my_feature` branch as before, and run the `push` command again. Conveniently, this will automatically push the changes to the same work-in-progress PR at Caiman. Note that if your PR is open for too long, merge conflicts can start to emerge as the `dev` branch changes. | ||
You may be asked to make some changes (or to *think* about making some changes). You will sometimes need to do more some more work on your branch and make more changes after making an initial PR. In this case, the workflow is simple: you will work within your your local `my_feature` branch as before, and run the `push` command again. Conveniently, this will automatically push the changes to the same work-in-progress PR at Caiman. Eventually, the feature will be merged into `dev` and your work is done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually, if accepted, the branch will be merged into dev ...
(we don't want to imply that doing the work means it will be accepted)
## Fourth, wait for the work to show up in main | ||
Once your work is done, it will eventually be integrated into `main` by the developers who maintain Caiman (label 4 in the figure). This is done every month or two, and is the stage when your work will actually be visible to people who download Caiman. It's at this point your name will appear when you click on the [list Contributors](https://github.com/flatironinstitute/CaImAn/graphs/contributors) at Github -- time to give yourself a big pat on the back! We really appreciate the folks who go through all the work to help make the package better. | ||
## Fourth, wait for the work to show up in main :clock8: | ||
Once your work is done, the `dev` branch will eventually be merged into `main` by the developers who maintain Caiman (label 4 in the figure). This is done every month or two, and is the stage when your work will actually be available to the people who download Caiman. It's at this point your name will appear when you click on the [list of Contributors](https://github.com/flatironinstitute/CaImAn/graphs/contributors) at GitHub. Please give yourself a pat on the back -- we really appreciate the folks who go through all this work to help make the package better! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can omit the pep-talk stuff in the last sentence?
|
||
Nobody remembers all the git commands, don't worry if you constantly are looking things up: that's what we all do. If you want to learn more, check out the following resources: | ||
Nobody remembers all the git commands, don't worry if you constantly are looking things up: that's what *everyone* does. If you want to learn more, check out the following resources: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to bend over backwards so far to tell people it's okay not to know everything? Maybe we can just say that once at the top and not repeat it so much
|
||
## Fourth, wait for the work to show up in main | ||
Once your work is done, it will eventually be integrated into `main` by the developers who maintain Caiman (label 4 in the figure). This is done every month or two, and is the stage when your work will actually be visible to people who download Caiman. It's at this point your name will appear when you click on the [list Contributors](https://github.com/flatironinstitute/CaImAn/graphs/contributors) at Github -- time to give yourself a big pat on the back! We really appreciate the folks who go through all the work to help make the package better. | ||
## Fourth, wait for the work to show up in main :clock8: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not to have more emoji invade the docs
Ok made some changes based on comments -- I'm ok with emojis and cheering people on, seems good to me. |
I don't want more emojis in our docs. I don't like seeing them, they look unprofessional to me, and I don't expect this to change. Please remove them from this PR. |
Description
Minor updates to contributors docs: show how to add upstream branch, and cleaned up some of the jargon/syntax throughout.