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

Remove unused temp variable #246

Merged

Conversation

neilenns
Copy link
Contributor

@neilenns neilenns commented Jun 3, 2023

Fixes #245

Removes the unused temp variable. The variable was introduced here.

@neilenns neilenns requested a review from DocMoebiuz as a code owner June 3, 2023 17:39
@neilenns neilenns requested a review from GioCC June 3, 2023 17:40
@github-actions
Copy link

github-actions bot commented Jun 3, 2023

Firmware for this pull request:
Firmware.zip

src/Config.cpp Outdated
uint16_t addreeprom = MEM_OFFSET_CONFIG;
uint16_t length = MFeeprom.get_length();
configLength = 0;

while (MFeeprom.read_byte(addreeprom++) != 0x00) {
configLength++;
if (addreeprom > length) // abort if EEPROM size will be exceeded
if (addreeprom > length) // abort if EEPROM size will be exceeded
Copy link
Collaborator

Choose a reason for hiding this comment

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

comment alignment. probably due to auto formatting rule.

This comment is also not necessary because it's obvious what is happening.

if length was called eepromLength it would be even more clear.

In general I propose to put comments above the line instead of behind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want me to remove the comment as part of this PR? I was only trying to remove the temp variable, I didn't bother touching anything else.

Copy link
Collaborator

Choose a reason for hiding this comment

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

we can remove it - thanks

src/Config.cpp Outdated Show resolved Hide resolved
@neilenns neilenns requested a review from DocMoebiuz June 4, 2023 18:15
@github-actions
Copy link

github-actions bot commented Jun 4, 2023

Firmware for this pull request:
Firmware.zip

Copy link
Collaborator

@DocMoebiuz DocMoebiuz left a comment

Choose a reason for hiding this comment

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

I was not aware of the habit to have comments on end of line which makes our syntax formatter format this file every where. I don't like the long lines either, I will discuss it on #development

@DocMoebiuz DocMoebiuz merged commit e9b757c into MobiFlight:main Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

temp variable is unused in readConfigLength()
3 participants