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.

Sign up for GitHub

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

Jump to bottom

Fix MSVC compiler warnings #22701

Merged
merged 2 commits into from May 17, 2024
Merged

Fix MSVC compiler warnings #22701

merged 2 commits into from May 17, 2024

Conversation

Jojo-Schmitz
Copy link
Contributor

reg.: declaration hides class member (C4458)

@Jojo-Schmitz Jojo-Schmitz force-pushed the compiler-warnings branch 8 times, most recently from a23aa74 to e4c4b38 Compare May 15, 2024 17:23
@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented May 15, 2024 •
edited

What the heck is wrong with "CI_Without Qt" all of a sudden?

Processing triggers for fontconfig (2.13.1-2ubuntu3) ...
update-alternatives: error: alternative path /usr/bin/gcc-11 doesn't exist
Setup failed
Error: Process completed with exit code 1.

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented May 16, 2024 •
edited

Besides the GitHub CI error there's another issue:

Warning MSB8027 Two or more files with the name of ioc.cpp will produce outputs to the same location. This can lead to an incorrect build result. The files involved are ...\src\framework\global\thirdparty\kors_modularity\modularity\ioc.cpp, ...\src\framework\global\modularity\ioc.cpp.

@Jojo-Schmitz Jojo-Schmitz mentioned this pull request May 17, 2024
Closed
1 task
Eism
Eism reviewed May 17, 2024
View reviewed changes
src/playback/internal/playbackcontroller.cpp Outdated
}

void PlaybackController::setTempoMultiplier(double multiplier)
{
INotationPlaybackPtr playback = notationPlayback();
if (!playback) {
INotationPlaybackPtr pb = notationPlayback();
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, from my perspective, such variable names are not readable: if the method is large, then after a certain block of code you will no longer understand what ab/pb means

Copy link
Contributor Author

Choose a reason for hiding this comment

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

playback is taken though

And this method is rather small

as is the scope of that ab further up

Copy link
Contributor

Choose a reason for hiding this comment

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

And this method is rather small

This is temporary :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Better names? playbackPtr? autobotOptions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check now, whether you like it better...

@Jojo-Schmitz
Fix MSVC compiler warnings
dee0148 
reg.: declaration hides class member (C4458)
@Jojo-Schmitz
Fix "GitHub CI without Qt"
63e01f1
@Eism Eism merged commit 2e494cf into musescore:master May 17, 2024
11 checks passed
@Jojo-Schmitz Jojo-Schmitz deleted the compiler-warnings branch May 21, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@igorkorsukov igorkorsukov igorkorsukov approved these changes

@Eism Eism Eism approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Jojo-Schmitz @igorkorsukov @Eism