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

Fix typo-s in robot class #1004

Merged
merged 2 commits into from
Jul 17, 2015

Conversation

sdimkov
Copy link
Contributor

@sdimkov sdimkov commented Jul 6, 2015

No description provided.

@sdimkov sdimkov force-pushed the fix-typos-in-robot-class branch from b0d6154 to a42e903 Compare July 6, 2015 16:00
@@ -392,7 +392,6 @@ class Robot
# Returns nothing.
parseHelp: (path) ->
@logger.debug "Parsing help for #{path}"
scriptName = Path.basename(path).replace /\.(coffee|js)$/, ''
Copy link
Collaborator

Choose a reason for hiding this comment

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

#917 actually depends on this being present. In addition, I'd much prefer to have test cases in place before modifying this code.

@michaelansel
Copy link
Collaborator

Please remove the code change per my comments and I'll merge

@sdimkov sdimkov force-pushed the fix-typos-in-robot-class branch from a42e903 to e721aa2 Compare July 6, 2015 17:19
@sdimkov
Copy link
Contributor Author

sdimkov commented Jul 6, 2015

I've removed the commit.

Are you sure #917 will get merged in the near future? It's open for the last 3 months

I agree with you in general about the importance of tests and coverage.Yet in this particular case that argument makes very little sense.

technicalpickles added a commit that referenced this pull request Jul 17, 2015
@technicalpickles technicalpickles merged commit ca51098 into hubotio:master Jul 17, 2015
@technicalpickles
Copy link
Member

Are you sure #917 will get merged in the near future?

I'm not really sure of it. I left my feedback down in #917 (comment) there.

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