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

[DOC] Update CONTRIBUTING and README with developer installation, contributing and testing instructions #375

Merged
merged 6 commits into from
Aug 15, 2019

Conversation

jsheunis
Copy link
Contributor

Closes #346

Changes proposed in this pull request:

README:

  • Installation instructions were given more structure with headings. Divided into installation for:
    • local python environment,
    • conda environment
    • developer setup (which now links to updated CONTRUBTING, but could in future link to fully separate DEVELOPMENT_GUIDE à la scona)

CONTRIBUTING:

@codecov
Copy link

codecov bot commented Jul 26, 2019

Codecov Report

Merging #375 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #375   +/-   ##
=======================================
  Coverage   79.74%   79.74%           
=======================================
  Files          40       40           
  Lines        2236     2236           
=======================================
  Hits         1783     1783           
  Misses        453      453

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 03ec6a2...dfc4baa. Read the comment docs.

CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@jbteves jbteves left a comment

Choose a reason for hiding this comment

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

Just add the small correction from pip

CONTRIBUTING.md Outdated Show resolved Hide resolved
@emdupre
Copy link
Member

emdupre commented Aug 14, 2019

This is great, thanks @jsheunis ! 🌻

Just wanted to ping since it looks like there's only one small suggested revision, and I want to make sure this doesn't go stale. Let us know if we can help in any way !

@emdupre
Copy link
Member

emdupre commented Aug 14, 2019

@all-contributors please add @jsheunis for documentation !

@allcontributors
Copy link
Contributor

@emdupre

I've put up a pull request to add @jsheunis! 🎉

replaces `python setup.py develop` with `pip install -e`

Co-Authored-By: Joshua Teves <jbtevespro@gmail.com>
@jsheunis
Copy link
Contributor Author

I'm a bit new to the process of reviews, suggestions, revisions, etc. I committed the change that @jbteves suggested, hope that was the right thing to do and the right way to do it!

@jbteves
Copy link
Collaborator

jbteves commented Aug 14, 2019

@jsheunis that's fine. When a reviewer makes a code suggestion GitHub offers the option to automatically create a commit and add it to the branch with the reviewer as co-author. If you make and push an otherwise identical commit manually on your own machine then the reviewer won't be co-authored unless you add a tag. That's the only real difference. I seriously doubt anybody will get bent out of shape about it. The nice thing with a suggested change is that it's just one click to add it if the original contributor agrees.

@emdupre
Copy link
Member

emdupre commented Aug 14, 2019

This is great, thanks @jsheunis ! Committing the suggested change is perfect in my book.

@jbteves can you update your review if you're ok with it as-is and we'll get this merged ?

Copy link
Collaborator

@jbteves jbteves left a comment

Choose a reason for hiding this comment

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

LGTM

@emdupre emdupre merged commit f75d6fa into ME-ICA:master Aug 15, 2019
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.

Update CONTRIBUTING with detailed process for dev setup, branching, submitting PRs
5 participants