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

Deprecated getLanguageName. Added getPluginName. #23

Merged
merged 3 commits into from
Sep 13, 2015

Conversation

WaDelma
Copy link
Contributor

@WaDelma WaDelma commented Sep 3, 2015

Fixes issue #14

@@ -27,10 +27,18 @@
public interface LanguagePlugin {

/**
* Returns the name of the plug-in
*
* @return The name of the plug-in
Copy link
Member

Choose a reason for hiding this comment

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

We can drop the @returns part of the javadoc

@jamo
Copy link
Member

jamo commented Sep 3, 2015

LGTM, please fix the two comments

@WaDelma
Copy link
Contributor Author

WaDelma commented Sep 7, 2015

Is everything fine with this commit? It decreases code coverage, because it adds a method that replaces old one. If the tests were changed to use new one it still would do that.

@@ -27,10 +27,16 @@
public interface LanguagePlugin {

/**
* Returns the name of the plug-in.
*
Copy link
Member

Choose a reason for hiding this comment

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

Rm blank line

@jamo
Copy link
Member

jamo commented Sep 7, 2015

Added one last comment.

We don't get notified when you push new changes to the pr...

@WaDelma
Copy link
Contributor Author

WaDelma commented Sep 7, 2015

Yeah I realised it from an irc conversation 😄

jamo added a commit that referenced this pull request Sep 13, 2015
Deprecated getLanguageName and added getPluginName.
@jamo jamo merged commit 504c586 into testmycode:master Sep 13, 2015
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.

None yet

2 participants