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

Musicxml import logging #3330

Merged
merged 4 commits into from Nov 15, 2017

Conversation

Projects
None yet
3 participants
@lvinken
Copy link
Contributor

commented Nov 12, 2017

MusicXML import refactoring: moved some common code between pass 1 and 2 to a separate class.

@Jojo-Schmitz

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2017

testutils don't build and therefore the mtests fail

@lvinken

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2017

Thanks, should be fixed now

@lasconic

This comment has been minimized.

Copy link
Member

commented Nov 15, 2017

I will try the merge with squash on this PR.

@lasconic lasconic merged commit b609e87 into musescore:master Nov 15, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lasconic

This comment has been minimized.

Copy link
Member

commented Nov 15, 2017

It doesn't look too bad !
https://github.com/musescore/MuseScore/commits/master

Regarding the PR, good to have separate logger for musicxml import!

class MxmlLogger {
public:
enum class Level : char {
TRACE, INFO, ERROR

This comment has been minimized.

Copy link
@Jojo-Schmitz

Jojo-Schmitz Nov 15, 2017

Contributor

INFO and ERROR conflicts with something in a Windows specific header, so it doesn't build there, renaming to XINFO and XERROR fixes the build problem

@lasconic

This comment has been minimized.

Copy link
Member

commented Nov 15, 2017

Maybe this whole logging system could be replaced by https://blog.qt.io/blog/2014/03/11/qt-weekly-1-categorized-logging/ ?

@lvinken

This comment has been minimized.

Copy link
Contributor Author

commented Nov 18, 2017

Actually in my opinion we could use both the qC* logging (selectively enable logging to debug user issues) and reporting of the import result in the GUI instead of as console output. When designing my logging facility I was targeting the second case. As all of the importer's output now goes through a dedicated class, it is easy to change where it goes to.

@lvinken lvinken deleted the lvinken:musicxml-import-logging branch Nov 18, 2017

nickless81 pushed a commit to nickless81/MuseScore that referenced this pull request Nov 22, 2017

Musicxml import logging (musescore#3330)
Create separate logger class for MusicXML import

lasconic added a commit that referenced this pull request Dec 6, 2017

Musicxml import logging (#3330)
Create separate logger class for MusicXML import

blackears added a commit to blackears/MuseScore that referenced this pull request Jul 4, 2018

Musicxml import logging (musescore#3330)
Create separate logger class for MusicXML import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.