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

Add and use StringToIntOrDefault #590

Merged
merged 13 commits into from
Jan 4, 2015

Conversation

peternewman
Copy link
Member

No description provided.

@nomis52
Copy link
Member

nomis52 commented Jan 3, 2015

LGTM

@peternewman
Copy link
Member Author

I'll skip enabling strict for now I think.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0%) when pulling 2dd7d88 on peternewman:libusb into 47721de on OpenLightingProject:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) when pulling 46238b0 on peternewman:libusb into 47721de on OpenLightingProject:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling 6172607 on peternewman:libusb into 47721de on OpenLightingProject:master.

@@ -133,15 +133,12 @@ bool FtdiDmxPlugin::SetDefaultPreferences() {


/**
* Return the frequency as specified in the config file.
* @brief Return the frequency as specified in the config file or the default if
* invalid
*/
int unsigned FtdiDmxPlugin::GetFrequency() {
Copy link
Member

Choose a reason for hiding this comment

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

Does this still need to be a separate method now?

Copy link
Member Author

Choose a reason for hiding this comment

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

No I guess not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Should even provide a minor optimisation.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling e6fb211 on peternewman:libusb into 7f3f386 on OpenLightingProject:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling e6fb211 on peternewman:libusb into 7f3f386 on OpenLightingProject:master.

@@ -21,8 +21,8 @@ common_protocol_libolaproto_la_CXXFLAGS = $(COMMON_CXXFLAGS_ONLY_WARNINGS)

common_libolacommon_la_LIBADD += common/protocol/libolaproto.la

common/protocol/Ola.pb.cc common/protocol/Ola.pb.h: common/protocol/Ola.proto
common/protocol/Ola.pb.cc common/protocol/Ola.pb.h: common/protocol/Makefile.mk common/protocol/Ola.proto
Copy link
Member

Choose a reason for hiding this comment

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

I seem to remember removing this because it was causing a lot of things to get rebuilt on each pass through autoconf. That doesn't make sense though as the Makefile.mk should be static.

Can you just confirm?

Copy link
Member Author

Choose a reason for hiding this comment

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

I started investigating this because include/ola/plugin_id.h kept getting rebuilt while tweaking the doxygen files stuff.

I can confirm it only gets rebuilt if the files listed are touched and not if Makefile is.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) when pulling 15365cc on peternewman:libusb into 7f3f386 on OpenLightingProject:master.

@nomis52
Copy link
Member

nomis52 commented Jan 4, 2015

LGTM

peternewman added a commit that referenced this pull request Jan 4, 2015
Add and use StringToIntOrDefault
@peternewman peternewman merged commit dcae552 into OpenLightingProject:master Jan 4, 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

3 participants