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

Previous pull request minus CI #9

Closed
wants to merge 23 commits into from

Conversation

linwoodc3
Copy link
Contributor

@linwoodc3 linwoodc3 commented Jul 25, 2017

This PR fixes the bug and adds exception handling. Unittest instructions included.

Regarding the CI, it wouldn't have added anything to the library download (did you put this on pypi?). CI is just a way to ensure that libraries/code maintain their functionality when people add features to it. Just makes sure the code still works when anyone adds new lines of code. Good at pointing finger :-).

Closes #6 and #7

linwoodc3 added 23 commits July 23, 2017 10:28
… and exception handling for passing empty string and invalid ticker symbol
…ling, adds unittests, and includeds contin. integration tests for Windows, Linux, OSX.
commit c1fcdae
Merge: fde97ac fd73447
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 14:08:59 2017 -0400

    Merge branch 'urlFix_linwoodError' of https://github.com/linwoodc3/good-morning into urlFix_linwoodError

commit fde97ac
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 10:28:18 2017 -0400

    Fixes petercerno#6 and petercerno#7 url problems, adds exception handling, adds unittests, and includeds contin. integration tests for Windows, Linux, OSX.

commit fd73447
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 13:36:25 2017 -0400

    Fixing the travis ci build. # 17

commit f58f591
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 13:17:00 2017 -0400

    Fixing the travis ci build. # 16

commit fcc539a
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 13:06:44 2017 -0400

    Fixing the travis ci build. # 15

commit 6fc4e9f
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 12:55:46 2017 -0400

    Fixing the travis ci build. # 14

commit 6d04c78
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 12:40:49 2017 -0400

    Fixing the travis ci build. # 13

commit a280acf
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 12:20:20 2017 -0400

    Fixing the travis ci build. # 12

commit 57abfd1
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 12:12:17 2017 -0400

    Fixing the travis ci build. # 11

commit 536ec5d
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 11:51:13 2017 -0400

    Fixing the travis ci build. # 9

commit 7e8a6d7
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 11:39:51 2017 -0400

    Fixing the travis ci build. # 8

commit 7e95db7
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 11:33:16 2017 -0400

    Fixing the travis ci build. # 7

commit d97535a
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 11:23:15 2017 -0400

    Fixing the travis ci build. # 6

commit 7b48720
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 11:19:08 2017 -0400

    Fixing the travis ci build. # 5

commit 64f6f12
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 11:09:12 2017 -0400

    Fixing the travis ci build. # 4

commit c8501e9
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 10:56:42 2017 -0400

    Fixing the travis ci build. # 3

commit 7853d38
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 10:50:37 2017 -0400

    Fixing the travis ci build. # 2

commit 9a00fbf
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 10:47:04 2017 -0400

    Fixing the travis ci build.

commit 3df0634
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 10:28:18 2017 -0400

    Fixes petercerno#6 and petercerno#7 url problems.  Also adds unit test and exception handling for passing empty string and invalid ticker symbol
commit c1fcdae
Merge: fde97ac fd73447
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 14:08:59 2017 -0400

    Merge branch 'urlFix_linwoodError' of https://github.com/linwoodc3/good-morning into urlFix_linwoodError

commit fde97ac
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 10:28:18 2017 -0400

    Fixes petercerno#6 and petercerno#7 url problems, adds exception handling, adds unittests, and includeds contin. integration tests for Windows, Linux, OSX.

commit fd73447
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 13:36:25 2017 -0400

    Fixing the travis ci build. # 17

commit f58f591
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 13:17:00 2017 -0400

    Fixing the travis ci build. # 16

commit fcc539a
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 13:06:44 2017 -0400

    Fixing the travis ci build. # 15

commit 6fc4e9f
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 12:55:46 2017 -0400

    Fixing the travis ci build. # 14

commit 6d04c78
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 12:40:49 2017 -0400

    Fixing the travis ci build. # 13

commit a280acf
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 12:20:20 2017 -0400

    Fixing the travis ci build. # 12

commit 57abfd1
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 12:12:17 2017 -0400

    Fixing the travis ci build. # 11

commit 536ec5d
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 11:51:13 2017 -0400

    Fixing the travis ci build. # 9

commit 7e8a6d7
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 11:39:51 2017 -0400

    Fixing the travis ci build. # 8

commit 7e95db7
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 11:33:16 2017 -0400

    Fixing the travis ci build. # 7

commit d97535a
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 11:23:15 2017 -0400

    Fixing the travis ci build. # 6

commit 7b48720
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 11:19:08 2017 -0400

    Fixing the travis ci build. # 5

commit 64f6f12
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 11:09:12 2017 -0400

    Fixing the travis ci build. # 4

commit c8501e9
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 10:56:42 2017 -0400

    Fixing the travis ci build. # 3

commit 7853d38
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 10:50:37 2017 -0400

    Fixing the travis ci build. # 2

commit 9a00fbf
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 10:47:04 2017 -0400

    Fixing the travis ci build.

commit 3df0634
Author: Linwood Creekmore III <[email protected]>
Date:   Sun Jul 23 10:28:18 2017 -0400

    Fixes petercerno#6 and petercerno#7 url problems.  Also adds unit test and exception handling for passing empty string and invalid ticker symbol
@@ -20,31 +20,32 @@
"""Module for downloading financial data from financials.morningstar.com.
"""

from __future__ import with_statement
from __future__ import absolute_import
Copy link
Owner

Choose a reason for hiding this comment

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

Where is this used?

Copy link
Contributor Author

@linwoodc3 linwoodc3 Jul 26, 2017

Choose a reason for hiding this comment

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

I'll delete this; but I was going to make this code work with Python 2 eventually (next plan) for backwards compatibility.

http://python-future.org/quickstart.html


import good_morning as gm
import morningstar as gm
Copy link
Owner

Choose a reason for hiding this comment

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

Is this correct? I do not see any file named morningstar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, @petercerno, this code works. This isn't a file, but it's folder to hold a module. The __init__.py file in the folder is what makes the code inside the folder importable.

The CI test I had made sure it worked. And, even without the CI test, if you install this code, and run the example in the updated README, it works.

To give some info on the CI code, this is exactly what CI code is meant to support. I runs automatic tests when someone adds/pushes code to github.


from unittest import TestCase

from morningstar import good_morning as gm
Copy link
Owner

Choose a reason for hiding this comment

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

Is this import working? I don't see any file named morningstar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it works. If you install the library from my pull request, you should be able to run through the example. I added this folder in the last pull request a month ago.

Copy link
Owner

@petercerno petercerno left a comment

Choose a reason for hiding this comment

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

Thanks for PR. I have sent few comments. Would it be possible to do another PR which contains only one commit with the bug-fix? All those commits related to CI are totally confusing to me.

@linwoodc3
Copy link
Contributor Author

Thanks for reviewing !

Those commits are gone; they are just in the record. If you click the "View changes" button, that's all that changes.

Nothing else. Git just keeps a history of everything. This is ALL that changed: https://github.com/petercerno/good-morning/pull/9/files/a04d633cc63ae2ff7d9b9265e11c4f9d5a704c99

Red is code that is deleted. Green is code that is added.

All of the CI code is gone; it's just old history. You can see that I only added the bugfix and exception handling in the View changes page.

@linwoodc3
Copy link
Contributor Author

I'll just redo it. Don't want to confuse.

@linwoodc3 linwoodc3 closed this Jul 26, 2017
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.

Errors
2 participants