Skip to content

Conversation

@amorellgarcia
Copy link
Contributor

-Added methods logError, logWarn, logInfo to avoid using log(new Status(...))
-Added getBuildEnvironmentVariableBoolean() that returns the boolean value instead of String.
-Use that methods to remove duplicate code.

-Fix some File.equals(someString) that always returned false.
-Create a wrapper method (getBuildEnvironmentVariableBoolean) that
converts values to boolean from the getBuildEnvironmentVariable method.
-Create a wrapper method (getBuildEnvironmentVariable) that uses empty
string as default value.
-Use new methods to reduce code size.
Copy link
Member

Choose a reason for hiding this comment

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

Any reason why you changed the warning to an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, it's a mistake. Maybe I got confused by the message.
There is an option on github to modify the pull request?

El 24/12/13 18:57, jantje escribió:

In it.baeyens.arduino.common/src/it/baeyens/arduino/arduino/Serial.java:

@@ -62,7 +59,7 @@
* General error reporting, all correlated here just in case I think of something slightly more intelligent to do.
*/
static public void errorMessage(String where, Throwable e) {

  • Common.log(new Status(IStatus.WARNING, ArduinoConst.CORE_PLUGIN_ID, "Error inside Serial. " + where, e));
  • Common.logError("Error inside Serial. " + where, e);

Any reason why you changed the warning to an error?


Reply to this email directly or view it on GitHub
https://github.com/jantje/arduino-eclipse-plugin/pull/116/files#r8545373.

Copy link
Member

Choose a reason for hiding this comment

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

There is an option on github to modify the pull request?

I don't know. I never made an accepted pull request in github

@amorellgarcia
Copy link
Contributor Author

In a few days I will fix all issues/bugs commented and open another pull request.
I you see other issues, please let me know ;)

@jantje
Copy link
Member

jantje commented Dec 24, 2013

I've gone through the whole lot so I guess that is it.
Txs for putting time in this plugin

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.

2 participants