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

minor cleanup #1394

Merged
merged 3 commits into from
Dec 19, 2020
Merged

minor cleanup #1394

merged 3 commits into from
Dec 19, 2020

Conversation

digant73
Copy link
Contributor

@digant73 digant73 commented Dec 15, 2020

just a minor cleanup (not committed on my just merged PR)

fixes #1386

Comment on lines +150 to +170
if (heatGetTargetTemp(c_heater) < infoSettings.min_ext_temp)
{
labelChar(tempStr, LABEL_TUNE_EXT_TEMPLOW);

sprintf(tempMsg, tempStr, infoSettings.min_ext_temp);

popupReminder(DIALOG_TYPE_ALERT, tuneExtruderItems.title.index, (u8 *) tempMsg);
}
else if (heatGetCurrentTemp(c_heater) < heatGetTargetTemp(c_heater) - 1)
{
popupReminder(DIALOG_TYPE_ALERT, tuneExtruderItems.title.index, LABEL_TUNE_EXT_DESIREDVAL);
}
else
{
labelChar(tempStr, LABEL_TUNE_EXT_MARK120MM);

sprintf(tempMsg, tempStr, textSelect(LABEL_EXTRUDE));

setDialogText(tuneExtruderItems.title.index, (u8 *) tempMsg, LABEL_EXTRUDE, LABEL_CANCEL);
showDialog(DIALOG_TYPE_QUESTION, extrudeFilament, NULL, NULL);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you adding space after every line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I corrected the indentation of that piece of code starting from the original code. My pr was still on draft when merged. However, if any style guide will be defined/applied it must be applied by anyone working on the project and on the whole project otherwise it is simply a waste of time for me, you and everyone. It should be mandatory to enable and use a defined beautifier on vscode. It should be better to have a defined style guide from now on. But in the meanwhile I think it is better to limit changes only to clear wrong format. E.G. I usually always separate variables definition from code by a blank line etc._.. That is not a format violation but simply a personal preference I adopted and imposed in all of my teams just because it allows to make more uniform and more clear the code written by people otherwise using totally different approaches

Copy link
Contributor

Choose a reason for hiding this comment

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

I request you to not take it on you personally. Adding space to differentiate between assigning values and function calls is a good thing to do, but do you think it is good if adding line spaces results in space after every line or if there are just 2-3 lines per block? This results in less code lines being shown on the screen which makes it hard to analyse and edit the code. Instead wouldn't it be better to increase overall line spacing in the editor iteself.
Should we not keep the new code match the code already existing in the project? What if more contributors keep insisting on keeping their personal style on files added by them? even I have many personal preferences for writing codes which are not according to style for this opensource project but still use them on personal projects and I always keep trying not to incorporate them, but this should not mean that I should get bored or it becomes a waste of time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but I agree to use a common style (standard rules are present since C was born). The problem is that in the whole project I see different styles (often home made) even inside the same file. Currently there is no style for this opensource (it was open even a bug to discuss/adopt a style for the project). That was my explanation. If a style for this project is provided I would apply it. In the absence of that I simply said that it is a waste of time for all of us. It's not a blank line a violation to style guide but the problem relies in a mix (often home made) of styles used by a lot of (often occasional) contributors.

@bigtreetech bigtreetech merged commit 88629e5 into bigtreetech:master Dec 19, 2020
@digant73 digant73 deleted the bugfix-2020-12-15 branch May 31, 2021 14:12
jeffeb3 pushed a commit to V1EngineeringInc/BIGTREETECH-TouchScreenFirmware that referenced this pull request Nov 10, 2021
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.

[BUG] Onboard SD print - Layer display stops at x height
4 participants