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

Create a menu to show KA Lite logs #457

Merged
merged 4 commits into from Jun 16, 2017

Conversation

Projects
None yet
5 participants
@mrpau-richard
Member

mrpau-richard commented Jun 8, 2017

Summary

I created a new menu to show the KA Lite logs using LogExpet application.

Issues addressed

Fixes #455

@radinamatic Here's the windows installer to test this.

@mrpau-richard mrpau-richard requested a review from radinamatic Jun 8, 2017

@radinamatic

This comment has been minimized.

Show comment
Hide comment
@radinamatic

radinamatic Jun 8, 2017

Contributor

@mrpau-richard I wasn't aware this required additional .NET framework install:

logexpet1

It wasn't problematic to install, I'm just not sure we should require it from users...

logexpet2

Would this work on older versions of Windows? How about if they can't connect the device to Internet to download the .NET framework installer?

Could we try to implement the option for viewing logs without additional installation? What's wrong with plain ol' Notepad? They had that back on Win 95... 😉

@benjaoming Thoughts?

Contributor

radinamatic commented Jun 8, 2017

@mrpau-richard I wasn't aware this required additional .NET framework install:

logexpet1

It wasn't problematic to install, I'm just not sure we should require it from users...

logexpet2

Would this work on older versions of Windows? How about if they can't connect the device to Internet to download the .NET framework installer?

Could we try to implement the option for viewing logs without additional installation? What's wrong with plain ol' Notepad? They had that back on Win 95... 😉

@benjaoming Thoughts?

@benjaoming

This comment has been minimized.

Show comment
Hide comment
@benjaoming

benjaoming Jun 8, 2017

Member

I agree with @radinamatic - we shouldn't require external software for a trivial thing such as logs. The most important part is that users are aware that they exist and can easily open them and copy-paste stuff if they need to report about bugs etc.

Also, in case of crashes, it seems like a good idea to open up the log in Notepad so it won't disappear. In case of server.log it's not so dangerous in terms of file size because the log is truncated every time KA Lite is started.

Member

benjaoming commented Jun 8, 2017

I agree with @radinamatic - we shouldn't require external software for a trivial thing such as logs. The most important part is that users are aware that they exist and can easily open them and copy-paste stuff if they need to report about bugs etc.

Also, in case of crashes, it seems like a good idea to open up the log in Notepad so it won't disappear. In case of server.log it's not so dangerous in terms of file size because the log is truncated every time KA Lite is started.

mrpau-richard added some commits Jun 12, 2017

Merge branch 'master' into feature/445-create-a-logging-uI-in-kA-lite…
…-windows-application

# Conflicts:
#	windows/gui-packed/KA Lite.exe
#	windows/gui-source/KA Lite/KALite.cpp
#	windows/installer-source/KaliteSetupScript.iss
@mrpau-richard

This comment has been minimized.

Show comment
Hide comment
@mrpau-richard

mrpau-richard Jun 12, 2017

Member

@radinamatic thanks for testing this, I've removed the external application that's required .net framework and used notepad as you suggested. here's the installer for another testing.

Member

mrpau-richard commented Jun 12, 2017

@radinamatic thanks for testing this, I've removed the external application that's required .net framework and used notepad as you suggested. here's the installer for another testing.

@radinamatic

This comment has been minimized.

Show comment
Hide comment
@radinamatic

radinamatic Jun 12, 2017

Contributor

@mrpau-richard Perfect, Notepad is back! 👍

virtualbox_msedge - win10_preview_12_06_2017_14_22_24

Contributor

radinamatic commented Jun 12, 2017

@mrpau-richard Perfect, Notepad is back! 👍

virtualbox_msedge - win10_preview_12_06_2017_14_22_24

@radinamatic

Good to go!

@benjaoming

This comment has been minimized.

Show comment
Hide comment
@benjaoming

benjaoming Jun 12, 2017

Member

@mrpau-richard do you need a review from @cpauya? In case not, I recommend self-merging.

Member

benjaoming commented Jun 12, 2017

@mrpau-richard do you need a review from @cpauya? In case not, I recommend self-merging.

Show outdated Hide outdated windows/README.md Outdated
@mrpau-richard

This comment has been minimized.

Show comment
Hide comment
@mrpau-richard

mrpau-richard Jun 13, 2017

Member

@cpauya I'm done fixing all your comments.

Member

mrpau-richard commented Jun 13, 2017

@cpauya I'm done fixing all your comments.

@mrpau-eduard mrpau-eduard self-requested a review Jun 13, 2017

@cpauya

This comment has been minimized.

Show comment
Hide comment
@cpauya

cpauya Jun 13, 2017

Member

Nice one @mrpau-richard - let's test it one more time @mrpau-eduard then you can merge away.

Member

cpauya commented Jun 13, 2017

Nice one @mrpau-richard - let's test it one more time @mrpau-eduard then you can merge away.

@radinamatic

This comment has been minimized.

Show comment
Hide comment
@radinamatic

radinamatic Jun 13, 2017

Contributor

Ready to test again, if necessary! 👍

Contributor

radinamatic commented Jun 13, 2017

Ready to test again, if necessary! 👍

@mrpau-eduard

This comment has been minimized.

Show comment
Hide comment
@mrpau-eduard

mrpau-eduard Jun 14, 2017

Member

Hi @radinamatic you can grab the windows installer here. I built a new one for this feature so you can test it.
So far it is working on our side.

Member

mrpau-eduard commented Jun 14, 2017

Hi @radinamatic you can grab the windows installer here. I built a new one for this feature so you can test it.
So far it is working on our side.

@mrpau-eduard

This comment has been minimized.

Show comment
Hide comment
@mrpau-eduard

mrpau-eduard Jun 16, 2017

Member

Done testing this feature with @mrpau-eugene .
Merging this PR so we can use it :)

Member

mrpau-eduard commented Jun 16, 2017

Done testing this feature with @mrpau-eugene .
Merging this PR so we can use it :)

@mrpau-eduard mrpau-eduard merged commit 97850a7 into learningequality:master Jun 16, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment