Skip to content

Don’t keep important settings in Sparkle’s Info.plist#574

Merged
kornelski merged 2 commits into
masterfrom
unbundle
Jul 23, 2015
Merged

Don’t keep important settings in Sparkle’s Info.plist#574
kornelski merged 2 commits into
masterfrom
unbundle

Conversation

@kornelski

Copy link
Copy Markdown
Member

Re #573

@zorgiepoo

Copy link
Copy Markdown
Member

This seems like a good idea to me. sparkleBundle property in SUHost is completely useless/unused now (and technically wrong anyway). It should be removed.

Can we use -[NSBundle bundleForClass:] for SULocalizedString()? Just pick a class (e.g.: SUHost) that's shared between Sparkle.framework and Autoupdate.app. I presume Autoupdate.app has its own copy of string files.

@zorgiepoo

Copy link
Copy Markdown
Member

OK, I was wrong about sparkleBundle not being used. The code seems fine considering where -userAgentString and -installWithToolAndRelaunch:displayingUserInterface: is being called, but I am wary about sparkleBundle being a property of SUHost, when [NSBundle bundleForClass:[self class]] can just be executed outside.

@kornelski

Copy link
Copy Markdown
Member Author

Autoupdate.app doesn't have its own strings files. I'd rather not split them, as this makes strings maintenance harder.

@kornelski

Copy link
Copy Markdown
Member Author

OK, SUUpdater seems like a better fit for the bundle

@zorgiepoo

Copy link
Copy Markdown
Member

What are all the .strings files I see generated inside of Autoupdate.app for? I see that some of the resource files are copied over from Sparkle.framework to Autoupdate.app perhaps maybe to avoid this problem

If you grep for [NSBundle bundleForClass:[self class]] you can see it's already being used in SUSystemProfiler.m too. SUUpdater.m and SUBasicUpdateDriver.m would be to affected files for us.

@kornelski

Copy link
Copy Markdown
Member Author

What are all the .strings files I see generated inside of Autoupdate.app for

Oh, I didn't notice there were there. It's a dupe of framework's strings files, so hopefully it could be avoided? Seems wasteful.

If you grep for [NSBundle bundleForClass:[self class]] you can see it's already being used in SUSystemProfiler.m

That class is currently independent of SUUpdater, and that call works, so I'd leave it as is.

kornelski added a commit that referenced this pull request Jul 23, 2015
Don’t keep important settings in Sparkle’s Info.plist
@kornelski kornelski merged commit 382b04f into master Jul 23, 2015
@kornelski kornelski deleted the unbundle branch July 23, 2015 23:11
@zorgiepoo

Copy link
Copy Markdown
Member

Oh, I didn't notice there were there. It's a dupe of framework's strings files, so hopefully it could be avoided? Seems wasteful.

It may seem 'wasteful' but could help save us from this issue, which I think would be worth it.

@michelf

michelf commented Jul 24, 2015

Copy link
Copy Markdown
Contributor

If it's too wasteful to have two copies, you can put only one copy in Autoupdate and have a relative symlink in the framework that points to the file in Autoupdate.

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.

3 participants