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

Removed static classes #489

Open
wants to merge 10 commits into
base: development
Choose a base branch
from
Open

Removed static classes #489

wants to merge 10 commits into from

Conversation

rfaes
Copy link
Contributor

@rfaes rfaes commented Aug 3, 2016

Removed static classes and injecting the classes with constructor injection.
Next steps could be:

  • clean up tasks, quite a lot of duplicated code
  • write unit test for important services/tasks
  • .... there is enough to do ;)

@bmdr38
Copy link
Contributor

bmdr38 commented Aug 3, 2016

Does git have a hook for running tests on commit/push/pr? I have also thought about adding tests for things, but after working in the real world, no one manually runs tests - there are systems in place to do that for you before submitting/while review iterating/etc. Otherwise I foresee those tests going ignored soon after being introduced

@rfaes
Copy link
Contributor Author

rfaes commented Aug 3, 2016

Github doesn't as far as I know. We could link a the repo to a tfs online server and build it and run the tests there, but it's not free :/
There was an option like that in the past, but I'm not able to find it in visual studio 2015.

I will resolve the conflicts tonight.

@DurtyFree
Copy link
Member

Awesome, you took quite off some work from me:)
Please fix the outstanding conflicts @rfaes

@serginator
Copy link

About running tests on commit/push/pr you should add some hooks, maybe to run a install.sh which generates some hooks under .git/hooks, pre-commit and pre-push, and maybe using travis-ci to check PRs (https://docs.travis-ci.com/user/languages/cpp). Hope it helps :)

@littlemonster1
Copy link
Contributor

littlemonster1 commented Aug 3, 2016

not sure but after reading up on static and checking the file, this might also solve #434 :)

@bin101
Copy link
Contributor

bin101 commented Aug 3, 2016

https://www.appveyor.com/ could do automatic build checks and testing.
And it is free for open-source projects

@jodendaal
Copy link

I've used myget previously. It was prety good.
http://docs.myget.org/docs/reference/build-services

@rfaes
Copy link
Contributor Author

rfaes commented Aug 3, 2016

There are now cyclical dependencies again -.-
ApiFailureStrategy and Client are the trouble makers. I try to fix it now.

I don't get usable pokestops close to me, can somebody please check?

@Jazed
Copy link
Contributor

Jazed commented Aug 6, 2016

Your PR has conflicts, please solve.

@Jazed Jazed closed this Aug 6, 2016
@rfaes
Copy link
Contributor Author

rfaes commented Aug 7, 2016

Removed conflicts. Can't really test it at the moment...
@Jazed please reopen pr.

@Jazed Jazed reopened this Aug 7, 2016
@rfaes
Copy link
Contributor Author

rfaes commented Aug 7, 2016

guess that has nothing to do with the branch ;)
Could not find .travis.yml, using standard configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants