Skip to content

Conversation

@CJuette
Copy link
Contributor

@CJuette CJuette commented Jul 21, 2018

This is the PR to Issue #20

CJuette added 2 commits July 21, 2018 09:36
This saves some memory since concatenating Strings means copying.
Also see Issue devinaconley#20
This is also to save some memory.
See Issue devinaconley#20
Copy link
Owner

@devinaconley devinaconley left a comment

Choose a reason for hiding this comment

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

Looks good, thanks.

Just a style comment, can you add spaces into method calls?

@CJuette
Copy link
Contributor Author

CJuette commented Jul 27, 2018

I'm not quite sure what you mean by that - do you mean separating the print-statements into several lines?

bool config = counter == 0;

Serial.print( "{\"" + TIME_KEY + "\":" ); Serial.print( millis() );
Serial.print( "{\""); Serial.print(TIME_KEY); Serial.print("\":" ); Serial.print( millis() );
Copy link
Owner

Choose a reason for hiding this comment

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

@Chritzel - for example this line would be:

Serial.print( "{\"" ); Serial.print( TIME_KEY ); Serial.print( "\":" ); Serial.print( millis() );

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, added it.

@CJuette
Copy link
Contributor Author

CJuette commented Aug 1, 2018

Don't know if you saw this, I added two commits fixing the style :)

Copy link
Owner

@devinaconley devinaconley left a comment

Choose a reason for hiding this comment

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

+1 looks good

@devinaconley devinaconley merged commit 48fb97e into devinaconley:master Aug 3, 2018
@devinaconley
Copy link
Owner

devinaconley commented Aug 3, 2018

this closes #20

thanks @Chritzel !

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