constructor
constructor copied to clipboard
conda
Use proper logging mechanism instead of notify on macOS
The notifications often go unnoticed if a focus mode is active.
And if they don't go unnoticed, we found they're irritating to users, as clicking on them spawns a script editor.
Lastly, the emitted messages are not logged inside the installer.
To resolve these issues, the changes here replace all notify calls with proper logger invocations.
We require contributors to sign our Contributor License Agreement and we don't have one on file for @hoechenberger.
In order for us to review and merge your code, please e-sign the Contributor License Agreement PDF. We then need to manually verify your signature. We will ping the bot to refresh the PR status when we have confirmed your signature.
I added the notifications because we got feedback from users that thought the installer had hung. I get your point about intrusiveness and lack of logs, so I'd propose if we can make this an option? Something like:
osx_pkg_progress_notifications: true # default is false
The normal echo calls are logged too, by the way. I am not sure if using logger is advantageous in other ways, though.
The normal
echocalls are logged too, by the way. I am not sure if usingloggeris advantageous in other ways, though.
Are you sure about that? IIRC only with those logger calls I could properly see all messages in the GUI installer log window. But I'd have to check again.
I added the notifications because we got feedback from users that thought the installer had hung. I get your point about intrusiveness and lack of logs, so I'd propose if we can make this an option? Something like:
osx_pkg_progress_notifications: true # default is false
I've received user feedback that the notifications were confusing, as clicking on them opens the AppleScript editor.
It would be great if we could find a way to display "proper" notifications somehow and not have to abuse AppleScript for this stuff…
@hoechenberger Would it be ok if I push some changes to your branch introducing the configuration option to enable notifications (off by default)? This would be the ideal case for a plugin, but we don't have support for that (maybe in the future?), so one more config key would have to do now... If you prefer I open a 2nd PR as an alternative implementation, that's ok too!
@jaimergp Please feel free to push changes here, I'm currently busy with other stuff and therefore didn't have the time to come back to this PR
I think new commits should always refer to macOS (which is the correct spelling). We should do a separate PR to fix existing spelling…
Hm, we have a news/ folder but also a CHANGELOG.rst. I think the CHANGELOG is only updated by the release automations though, so we should turn the changes in CHANGELOG into a news piece :D I'll go for it.