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

Quit exit if failing to save project #4428

Merged
merged 2 commits into from Jun 19, 2018

Conversation

Projects
None yet
2 participants
@zonkmachine
Member

zonkmachine commented Jun 16, 2018

Test for, and return the result of, Engine::getSong()->guiSaveProject() when saving a project.

Fixes #4426

@@ -1022,8 +1024,7 @@ bool MainWindow::saveProjectAsNewVersion()
do VersionedSaveDialog::changeFileNameVersion( fileName, true );
while ( QFile( fileName ).exists() );
Engine::getSong()->guiSaveProjectAs( fileName );
return true;
return Engine::getSong()->guiSaveProjectAs( fileName );

This comment has been minimized.

@zonkmachine

zonkmachine Jun 16, 2018

Member

I'm not sure how to make MainWindow::saveProjectAsNewVersion() return false for the purpose of testing, but the function returns a bool and this looks correct. Still works.

@zonkmachine

zonkmachine Jun 16, 2018

Member

I'm not sure how to make MainWindow::saveProjectAsNewVersion() return false for the purpose of testing, but the function returns a bool and this looks correct. Still works.

This comment has been minimized.

@zonkmachine

zonkmachine Jun 16, 2018

Member

Easy to test. Just create a directory with test files and change ownership to root. Load and change one of the files and then saveAsNewVersion/(). Works as expected both with and without this change.

@zonkmachine

zonkmachine Jun 16, 2018

Member

Easy to test. Just create a directory with test files and change ownership to root. Load and change one of the files and then saveAsNewVersion/(). Works as expected both with and without this change.

@PhysSong

This comment has been minimized.

Show comment
Hide comment
@PhysSong

PhysSong Jun 17, 2018

Member

Looks good for me.
Just a reminder for branch synchronization: #4044 has changed the same part.

Member

PhysSong commented Jun 17, 2018

Looks good for me.
Just a reminder for branch synchronization: #4044 has changed the same part.

@zonkmachine

This comment has been minimized.

Show comment
Hide comment
@zonkmachine

zonkmachine Jun 17, 2018

Member

OK. I need to test this some more though.

Member

zonkmachine commented Jun 17, 2018

OK. I need to test this some more though.

@zonkmachine zonkmachine merged commit 583e42e into LMMS:stable-1.2 Jun 19, 2018

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@zonkmachine zonkmachine deleted the zonkmachine:savefailquitexit branch Jun 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment