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

Fix crash on start of Example app #107

Open
wants to merge 2 commits into
base: v2.11
Choose a base branch
from

Conversation

gpotari
Copy link
Contributor

@gpotari gpotari commented Aug 1, 2022

Fixes Building Example project.
Added missing short version string key to info.plist.

Also wanted to ask It is referencing a large-archive.rar that is not present in the repo. I can see the reason why, just wanted to ask just in case...

@abbeycode abbeycode changed the title Fixed Compiling Example project Fix crash on start of Example app Aug 1, 2022
Copy link
Owner

@abbeycode abbeycode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out! I have a CI check that makes sure it always builds, but I don't run it often. I need to see if I can make a quick UI test that CI can run just to make sure it doesn't crash on startup.

I renamed this ticket and suggested an update to the CHANGELOG line (thanks for including that!) to reflect that it actually was compiling, it was just crashing on startup. Also would like to roll back the scheme upgrade check lines to keep the PR focused on what actually needed to change.

I also made some changes in the base branch to update the README and code to explain what's going on with Large Archive a little better. You should be able to click "Extract Large File" now and try it out.

CHANGELOG.md Outdated
@@ -5,6 +5,7 @@
* Fixed a header name conflict with Realm (Issue #90)
* Incorrect passwords in RAR5 archives now produce a specific and helpful error message (PR #101 - Thanks to [@gpotari](https://github.com/gpotari) for the idea and implementation!)
* Updated to v6.1.7 of UnRAR library (PR #103 - Thanks to [@gpotari](https://github.com/gpotari) )
* Fixed compile error for Example project. Added missing CFBundleShortVersionString key to Info.plist
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the CHANGELOG to reflect the actual nature of the problem being fixed (a crash, not a compilation error).

Suggested change
* Fixed compile error for Example project. Added missing CFBundleShortVersionString key to Info.plist
* Fix crash on start of Example app (PR #107 - Thanks to [@gpotari](https://github.com/gpotari))

@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<Scheme
LastUpgradeVersion = "1200"
LastUpgradeVersion = "1400"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Please revert

@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<Scheme
LastUpgradeVersion = "1200"
LastUpgradeVersion = "1400"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Please revert

@gpotari
Copy link
Contributor Author

gpotari commented Aug 1, 2022

Forgot to mention that I was building with latest Xcode beta 4. And it actually didn't compile, gave me the mentioned error.
Thank you for your review, going to fix them soon.

@abbeycode
Copy link
Owner

Ok, that makes more sense. Feel free to and everything to “doesn’t compile with Xcode 14”

@gpotari
Copy link
Contributor Author

gpotari commented Aug 2, 2022

Modified

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