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

Added more stuff #3

Closed
wants to merge 12 commits into from
Closed

Added more stuff #3

wants to merge 12 commits into from

Conversation

edchiou
Copy link

@edchiou edchiou commented Feb 14, 2013

  1. Added radiant and dire tower & rax methods
    I took your suggestion and wrote these methods to return a hash, the keys being tower names and values being :alive and :dead symbols. I could change the keys to be symbol as wells, but I want your feedback on this first.
  2. Added the tower and barracks modules for the above methods

P.S. Is it possible for you to add a TODO list so I can know what still needs working on?

@edchiou
Copy link
Author

edchiou commented Feb 17, 2013

Can I get a comment on this?

@nashby
Copy link
Owner

nashby commented Feb 17, 2013

@edchiou I gave it a second thought to it and decided to leave these methods as is. Could you please make a PR with this commit + specs? Sorry for that.

@edchiou
Copy link
Author

edchiou commented Feb 17, 2013

No problem, I'll submit a pull request as soon as possible.

@edchiou
Copy link
Author

edchiou commented Feb 19, 2013

I resubmitted the original towers & barracks status methods along with their specs.

@nashby
Copy link
Owner

nashby commented Feb 19, 2013

Could you please name these methods as radiant_towers_status and so on? Also it would be awesome if you squash commits into one. How to: http://blog.steveklabnik.com/posts/2012-11-08-how-to-squash-commits-in-a-github-pull-request

Added tower and rax methods to match.rb

fixed typo

Added tower & rax methods/specs

Redid tower & rax status methods

removed reqs
@edchiou
Copy link
Author

edchiou commented Feb 19, 2013

Renamed the methods, and tried squashing. Not sure if squashed correctly.

@nashby
Copy link
Owner

nashby commented Feb 19, 2013

@edchiou thanks, I've squashed it by myself and pushed to master.

@nashby nashby closed this Feb 19, 2013
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