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

Update MoveToMapPokemon to use events instead of logger (similar to #2867) #2881

Closed
wants to merge 64 commits into from

Conversation

chrisle
Copy link
Contributor

@chrisle chrisle commented Aug 7, 2016

Short Description:

Update MoveToMapPokemon to use events instead of logger. Same as PR 2867 but includes documentation and a refactor of the module.

Fixes:

  • Deprecation warnings emitted by MoveToMapPokemon.

@chrisle chrisle changed the title Dev Update MoveToMapPokemon to use events instead of logger (similar to #2867) Aug 7, 2016
@bixuanzju
Copy link
Contributor

This is better than mine 👍

@bigkraig
Copy link
Contributor

bigkraig commented Aug 7, 2016

maybe a squash and a rebase is in order, i can't apply this cleanly

@chrisle
Copy link
Contributor Author

chrisle commented Aug 7, 2016

I was just realizing that actually. Hold on.

# Move To map pokemon
self.event_manager.register_event(
'move_to_map_pokemon_fail',
parameters=('message')
Copy link
Member

Choose a reason for hiding this comment

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

this needs to be ('message',), otherwise the tuple isn't valid

@douglascamata
Copy link
Member

douglascamata commented Aug 7, 2016

👍

@TheSavior review this when you can, please.

Approved with PullApprove

@brantje
Copy link
Contributor

brantje commented Aug 7, 2016

Can you resolve the conflicts?

@chrisle
Copy link
Contributor Author

chrisle commented Aug 7, 2016

grr. Some help needed: How do I squish or rebase commits that aren't mine so I'm left with just my commits?

@douglascamata
Copy link
Member

@chrisle easier to clone the project again, copy over your files and open another PR, imho.

@chrisle
Copy link
Contributor Author

chrisle commented Aug 7, 2016

Yeah.. i was starting to think that. OK.. hold on.

@chrisle
Copy link
Contributor Author

chrisle commented Aug 7, 2016

Please disregard this pull request. The merge went south. See PR 2913 instead for cleaner pull request.

@bigkraig
Copy link
Contributor

bigkraig commented Aug 7, 2016

You can rebuild your local branch and force push it to github instead of opening a new pr

Also used rebase -i to squash the commits

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.

7 participants