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

Fix use of incorrect function const and add a few missing ones #2992

Merged
merged 1 commit into from Aug 16, 2019

Conversation

@vadi2
Copy link
Member

commented Aug 16, 2019

Brief overview of PR changes/additions

From LGTM:

A 'const' modifier on a member function return type is useless. It is usually a typo or misunderstanding, since the syntax for a 'const' function is 'int foo() const', not 'const int foo()'.

That's exactly what happened here (misunderstanding).

Motivation for adding to Mudlet

actually mark readonly functions as readonly.

Other info (issues closed, discussion etc)

Fixes this LGTM query: https://lgtm.com/query/1192113587356471581/

@vadi2 vadi2 requested review from Mudlet/core-cpp as code owners Aug 16, 2019

@add-deployment-links

This comment has been minimized.

Copy link

commented Aug 16, 2019

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

@Kebap
Kebap approved these changes Aug 16, 2019
@SlySven
Copy link
Member

left a comment

👍 As the probable source of many of those misunderstandings I approve...

@@ -559,7 +559,7 @@ const unsigned int Host::assemblePath()
return totalWeight;
}

const bool Host::checkForMappingScript()
bool Host::checkForMappingScript()

This comment has been minimized.

Copy link
@SlySven

SlySven Aug 16, 2019

Member

Actually - is this possibly a const function - i.e. does not alter the state of the class it is in (or accesses)?

This comment has been minimized.

Copy link
@vadi2

vadi2 Aug 16, 2019

Author Member

I thought it would be but it is not, because it records state :)

@vadi2 vadi2 merged commit 9c13f8f into Mudlet:development Aug 16, 2019

4 of 6 checks passed

LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No code changes detected
Details
CodeFactor No issues found.
Details
LGTM analysis: C/C++ 3 fixed alerts
Details
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@vadi2 vadi2 deleted the vadi2:fix-incorrect-cosnt branch Aug 16, 2019

wsdmatty added a commit to wsdmatty/Mudlet that referenced this pull request Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.