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

Remove nslog dependency #494

Merged
merged 3 commits into from
Apr 18, 2023
Merged

Remove nslog dependency #494

merged 3 commits into from
Apr 18, 2023

Conversation

mauricioszabo
Copy link
Contributor

There's no reason anymore for it to exist...

@DeeDeeG
Copy link
Member

DeeDeeG commented Apr 18, 2023

Just popping in to add whatever context I can:


I assume this was a very macOS-centric feature that caused all JavaScript console.log()s to also add to the Apple System Log, for whichever developers might find that tooling desirable to use?

Logs an error message to the Apple System Log facility.

https://developer.apple.com/documentation/foundation/1395275-nslog

This is another home-grown Atom thing... https://github.com/atom/node-nslog


Given the many obscure problems this has caused, I'm pretty inclined to yeet and delete it.

Lite approve ✅ 👍, since I can't think of a strong technical basis on which to advocate for or against this. (Nor have I tested this change for breakage, either.) Anecdotally, nslog has caused nothing but pain and confusion, as far as I can recall anyway.

And I doubt that anyone takes advantage of this printing to the Apple System Log on their mac? Would be interested to hear from anyone who does, but it'd have to be a really important feature to justify the headaches it brings (on all platforms, especially Windows!) just for this minor, macOS-only feature.

@mauricioszabo
Copy link
Contributor Author

Wonderful, @DeeDeeG. We can work like we did with i18n - we remove it, and if the removal causes problems/complains we re-add it later :)

@mauricioszabo mauricioszabo merged commit 6a7c3df into master Apr 18, 2023
@mauricioszabo mauricioszabo deleted the remove-nslog branch April 18, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants