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

Contributing #54

Merged
merged 3 commits into from Aug 9, 2015
Merged

Contributing #54

merged 3 commits into from Aug 9, 2015

Conversation

arilaakk
Copy link
Contributor

Added basic contributing steps. The other one can be discarded

@TeroFrondelius
Copy link
Member

You need to remove the "jebou" commit arilaakk@d362d78. This might the correct instructions: http://stackoverflow.com/questions/20765030/git-remove-a-commit-from-pull-request

Also I would like to see little more details on each numbered point. Maybe it's easy when somebody is showing you want to do and from where to download, but that is the exact information we want to have in the Contributing.rst document. The whole idea is that we would have a tutorial, which will guide a new contributor from github registration to the first pull request.

@ahojukka5
Copy link
Member

I have couple of improvements for this

  • 3&4: julia is also released as a binary version which should be recommended
  • 6: how to do, exactly, commands?
  • 8: git add . adds all files from current directory to commit, I prefer to use git add file1 file2 ... filen to avoid adding unwanted files. (Btw, how to remove accidentally added files from commit..)
  • 9: git commit is not actually moving the files anywhere, it just creates a commit ready to go, after that user must also push files to his own repository using git push or git push origin master depending how git is configured. After git push the repository is updated and pull request can be made.
  • finally, it should be instructed what to do when contributors are asking some changes to pull request before it can be merged

@TeroFrondelius
Copy link
Member

Hint: git reset is a really nice command before git commit, meaning if you accidentally add too many files you can use git reset to start over. Also nice command is git status, which shows you the added files before you commit them.

Also good habit is always do git pull before git commit. Then you don't need to do the unnecessary merge.

@ahojukka5
Copy link
Member

A little off topic but anyway. Contributor should check that his own branch is most recent before making a pull request. We are having exactly this situation with Lexicon.jl, see https://github.com/JuliaFEM/Lexicon.jl (we're 4 commits behind). Maybe @ovainola could describe how to "refresh" outdated branch. "What to do if your branch is outdated?".

@ovainola
Copy link
Contributor

offtopic, Lexicon.jl is now updated. Here's an instructions, how to refresh outdated branch: https://help.github.com/articles/syncing-a-fork/

@ahojukka5
Copy link
Member

  • run tests before PR.

@arilaakk
Copy link
Contributor Author

Ok, I'll do the changes. Getting my first pull request under review was also part of the git learning process

@TeroFrondelius
Copy link
Member

Here is a related discussion about contributing: https://groups.google.com/forum/#!topic/julia-dev/thJpV8lrNYM

@arilaakk
Copy link
Contributor Author

arilaakk commented Aug 3, 2015

For number 4 (building Julia to the computer), I found good and precise instructions for all platforms at Julia readme (https://github.com/JuliaLang/julia/blob/master/README.md). Would it be best if I referred straight to it? Or does it contain too much information?

@arilaakk
Copy link
Contributor Author

arilaakk commented Aug 3, 2015

Ok so I did some modifications. The instructions are a lot more precise now, they basically cover everything I have done so far. I also added some styling.

@ahojukka5
Copy link
Member

I think this is good now.

@ovainola
Copy link
Contributor

ovainola commented Aug 4, 2015

I agree, It has all the major points covered

@TeroFrondelius
Copy link
Member

Done as requested.

@TeroFrondelius TeroFrondelius reopened this Aug 9, 2015
TeroFrondelius added a commit that referenced this pull request Aug 9, 2015
@TeroFrondelius TeroFrondelius merged commit eba7f5c into JuliaFEM:master Aug 9, 2015
@TeroFrondelius
Copy link
Member

Yesterday wrong clicks. Now this should be merged.

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