Skip to content
This repository has been archived by the owner on Mar 26, 2022. It is now read-only.

Changes made to prime numbers functions #3

Open
wants to merge 54 commits into
base: master
Choose a base branch
from

Conversation

teroanttila
Copy link

No description provided.

@juhovan
Copy link
Owner

juhovan commented Feb 23, 2016

Feedback:

  • You should have used branch for changes that you want to pull request. Because you didn't you are also requesting merging of Kasper's test branch (among other changes) as you have merged it to master. This is because how pull requests work, all changes to the branch you created pull request with will be added to the pull request.
    • Also you should use different branches and pull request for different features because if you get suggestions to fix something you can work with just one feature at once.
  • Fibonacci works as intended but the primes numbers are not working correctly. But don't spend time on fixing this unless you have time and want to :)
    • It seems you have code in place for this, I'm not going to use time to figure out why it's not working for me or review the code because it's not the goal of this course so don't worry about it.
  • Also please read the blog post about commit messages I linked in Tuubi :)
juho [12:45 PM] 
prime 5

showerBOT [12:45 PM] 
2,  3,  5,  7,  11,  13,  17,  19,  23,  29

[12:45] 
5 is prime number

[12:45] 
7 11 13 17 19 23 29 31 37 41

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.

5 participants