search

noseglid / atom-build

:hammer: Build your project directly from the Atom editor
https://atom.io/packages/build
MIT License
248 stars 97 forks source link

Make use of notification stack option in TargetManager #442

Closed Gawdl3y closed 8 years ago

Gawdl3y commented 8 years ago

There are several spots in the TargetManager that spit out Atom notifications for errors, that contain stack traces. These stack traces can be shown in the notification with the stack option, rather than appending them to the details. The notification will show a pretty little toggle-able stack trace.

See atom/notifications#126. It's most likely safe to use this option, since it's just a property containing a string - and that's probably not likely to change in the future.

noseglid commented 8 years ago

Looks much better. I'll await some response from https://github.com/atom/notifications/issues/126 to see if they strong discourage it, otherwise I'm happy to merge this!

Gawdl3y commented 8 years ago

Seems it's safe, as one of the Atom team members has submitted a PR to document the options already. atom/atom#11942

ssorallen commented 8 years ago

@Gawdl3y I'm not an Atom team member. I work on Nuclide. stack has been part of the Notification API, albeit undocumented, since v0.64.1 (https://github.com/atom/notifications/commit/c8bd7e3ea2edf684a5404acd592e8c6ff2f61cef#diff-35b222deaf95f037efa62f1169b0e6d4). It seems safe to use it to me, which is why I added it in that pull request. If you're concerned though, you might want to wait for an Atom team member to comment and accept the PR.

Gawdl3y commented 8 years ago

@ssorallen Ah, I just saw that you're a part of the Atom organization on your profile.

noseglid commented 8 years ago

Awesome 👍