esl / erlang-osx-installer

Erlang Installer & Auto-Updater for macOS
https://www.erlang-solutions.com/resources/download.html
Apache License 2.0
5 stars 5 forks source link

Saurabh #145

Closed nacicatus closed 6 years ago

nacicatus commented 6 years ago

Hi, I've refactored the code for Swift 4, so in branch "Saurabh" the code is free of errors but some warnings pertaining to the LaunchServices frawmeworks persist. I had to revert some code that I had previously committed from 10.13 to 10.11 because Xcode started to update itself and I had to switch to Xcode 8. So, there is still room for improvement. Please review if it is worth merging with master. The version 1.0.2 on your main fork works fine and installed on macOS 10.13.6 without a glitch. So, I'm not supremely happy about making changes to working software :+1: but I'm happy to have had a chance to explore it's inner workings, and thus my contribution if any should be considered merely cosmetic. Speaking of cosmetic, one important change I made was to the "menu-bar-icon" image attributes, from Render As "Default" to "Template Image". This allows the stylized "E" icon to appear properly if the Dark Mode option is selected. Any feedback will be appreciated. Thanks!

HernanRivasAcosta commented 6 years ago

Review will take a bit, this one is pretty long.

nacicatus commented 6 years ago

It's possible some of the conflicts in errors arose from my having to switch between Xcode versions. Am recompiling now on Xcode 9.4.1 on macOS 10.13.6

ghost commented 6 years ago

+1

On Thu, 19 Jul 2018, 08:51 SaurabhSikka, notifications@github.com wrote:

@nacicatus commented on this pull request.

In ErlangInstaller/MainMenu.swift https://github.com/esl/erlang-osx-installer/pull/145#discussion_r203630966 :

@@ -150,11 +156,11 @@ class MainMenu: NSMenu, NSUserNotificationCenterDelegate, PopoverDelegate { self.statusItem = NSStatusBar.system().statusItem(withLength: NSVariableStatusItemLength) self.statusItem?.image = NSImage(named: "menu-bar-icon.png") self.statusItem?.menu = self

  • if (UserDefaults.firstLaunch) {
  • popover.contentViewController = PopoverViewController(nibName: "PopoverViewController", bundle: nil)
  • = Timer.scheduledTimer(timeInterval: 1.0, target: self, selector: #selector(self.showPopover(:)), userInfo: nil, repeats: false)
  • UserDefaults.firstLaunch = false
  • }
  • if (UserDefaults.firstLaunch) {
  • popover.contentViewController = PopoverViewController(nibName: "PopoverViewController", bundle: nil)
  • = Timer.scheduledTimer(timeInterval: 1.0, target: self, selector: #selector(self.showPopover(:)), userInfo: nil, repeats: false)
  • UserDefaults.firstLaunch = false
  • }

agreed. I will attempt auto indent on all files before I commit

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/esl/erlang-osx-installer/pull/145#discussion_r203630966, or mute the thread https://github.com/notifications/unsubscribe-auth/Aego9hyEbKwufLlMSbkKgwKAVrErxD3Tks5uIDpsgaJpZM4VSZR- .

nacicatus commented 6 years ago

I've created another pull request free of conflicts and errors so closing this one