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

Cleanup social_handler.py #5245

Merged
merged 5 commits into from
Sep 6, 2016
Merged

Cleanup social_handler.py #5245

merged 5 commits into from
Sep 6, 2016

Conversation

sohje
Copy link
Contributor

@sohje sohje commented Sep 6, 2016

Short Description:

Social_handler cleanup

Fixes/Resolves/Closes (please use correct syntax):

  • pep8
  • cleanup
  • readability

@mention-bot
Copy link

@sohje, thanks for your PR! By analyzing the annotation information on this pull request, we identified @solderzzc, @BreezeRo and @Jasperrr91 to be potential reviewers

@Gobberwart
Copy link
Contributor

Why removed "need connect" and "connect again" debug messages?

@sohje
Copy link
Contributor Author

sohje commented Sep 6, 2016

Cause it doesnt bring any clue whats happening. This PR is first step to refactor classes and methods.

@Gobberwart
Copy link
Contributor

Gobberwart commented Sep 6, 2016

It certainly does bring a clue to what's happening... if the bot is constantly trying to reconnect, it can help to identify a connection issue.

Also, would like to see debug messages use self.bot.config.debug

if DEBUG_ON or self.bot.config.debug:

And of course use the logger to display messages.

@Gobberwart
Copy link
Contributor

Gobberwart commented Sep 6, 2016

Refer to #5216 why these connect messages are useful. If nothing else, leave them there for now so we can remember to replace them with something much better later :)

@sohje
Copy link
Contributor Author

sohje commented Sep 6, 2016

We can always check file history ;)

@sohje
Copy link
Contributor Author

sohje commented Sep 6, 2016

If we enable printing via self.bot.config.debug there will be a lot of stuff by api requests, response etc. So i left DEBUG_ON module global var.
I just reformat indentions, empty spaces, imports, etc. This is not refactoring PR :octocat: Just cleanup.

@Gobberwart
Copy link
Contributor

Agree, but both are needed, so that debug messages are printed either if DEBUG_ON == True, OR config.debug == True. Both should work.

In fact, probably easiest to just add:

if self.bot.config.debug: DEBUG_ON = True

to init section of each class

@Gobberwart
Copy link
Contributor

Gobberwart commented Sep 6, 2016

Anyway, for now please at least add connect messages back because they are actually helpful in some situations. Happy to approve once done.

@Gobberwart
Copy link
Contributor

Thanks. Approved.

@sohje
Copy link
Contributor Author

sohje commented Sep 6, 2016

Ok, thanks. Merge then.

@sohje sohje merged commit e9d1df6 into PokemonGoF:dev Sep 6, 2016
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.

3 participants