Skip to content

Conversation

JohnCaccavale
Copy link
Contributor

This PR Resolves issue #1391 where a macOS Command Line App will crash when the Parse Framework is installed in /Library/Frameworks/

…uld crash if the Parse Framework is installed in /Library/Frameworks, which results in the framework accidentally being filtered out and thus not having its subclasses properly registered.
@TomWFox
Copy link
Contributor

TomWFox commented Mar 17, 2019

Could anyone take a look at this? - @mrmarcsmith @jjmaceda @kennic

@mrmarcsmith mrmarcsmith self-requested a review March 17, 2019 22:42
@mrmarcsmith
Copy link

Thanks @JohnCaccavale for this PR! I know this may be difficult given the nature of this PR but can you think of any way to write a test to ensure current and future functionality?

@JohnCaccavale
Copy link
Contributor Author

Sorry, I've been very busy lately. Will try to take a look into that when I can get a chance.

@ShawnBaek
Copy link
Contributor

I wonder who is assignees and managing this repo?

@acinader
Copy link
Contributor

@ShawnBaek see #1356

@acinader acinader mentioned this pull request Mar 30, 2019
@TomWFox TomWFox added type:bug Impaired feature or lacking behavior that is likely assumed In Progress labels Apr 12, 2019
rodrigocarrapeiro pushed a commit to rodrigocarrapeiro/Parse-SDK-iOS-OSX that referenced this pull request Jan 15, 2020
rodrigocarrapeiro pushed a commit to rodrigocarrapeiro/Parse-SDK-iOS-OSX that referenced this pull request Jan 15, 2020
@JohnCaccavale JohnCaccavale changed the title macOS Command Line App Crash fix: macOS Command Line App Crash Jan 14, 2022
@parse-github-assistant
Copy link

parse-github-assistant bot commented Jan 14, 2022

Thanks for opening this pull request!

  • ❌ Please edit your post and use the provided template when creating a new pull request. This helps everyone to understand your post better and asks for essential information to quicker review the pull request.

@JohnCaccavale
Copy link
Contributor Author

@mtrezza I see this PR is still available. Please let me know if you would be able to review and if there's anything else that would be needed from me.

@mtrezza
Copy link
Member

mtrezza commented Jan 15, 2022

@mrmarcsmith could I add you to the team of Parse ObjC SDK reviewers? The group gets a notification when a PR is ready for review.

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

The only thing I can king of would be a test, if that is possible at all for this kind of change?

@mtrezza mtrezza removed the request for review from mrmarcsmith January 15, 2022 13:20
@JohnCaccavale
Copy link
Contributor Author

Yeah, I wasn't sure about how to setup a test for this type of scenario when I originally opened up the PR either. But, this fix had addressed my issue, which I've been using in my local branch since the PR was originally opened. Quite some time now. I would be concerned if this fix broke existing tests, which currently I'm seeing one macOS test failing, but not sure if thats a flaky test or not.

Failing tests:
	ParseUnitTests-macOS:
		-[AnonymousUtilsTests testLogInViaBlock]

@mtrezza
Copy link
Member

mtrezza commented Jan 16, 2022

Let's see whether the test passes in rerun, but I think this is good to merge also without test which may be difficult to implement.

@mtrezza
Copy link
Member

mtrezza commented Jan 16, 2022

The same test keeps failing, but I noticed that it also failed before this PR, so I think we are good to merge.

@mtrezza mtrezza requested a review from a team January 16, 2022 11:42
@mtrezza mtrezza changed the title fix: macOS Command Line App Crash fix: macOS command line app crash Jan 16, 2022
@mtrezza mtrezza removed the type:bug Impaired feature or lacking behavior that is likely assumed label Jan 19, 2022
@mtrezza
Copy link
Member

mtrezza commented Jul 26, 2022

I think we can merge this, but the carthage CI is currently failing, so we may have to address this first.

@JohnCaccavale
Copy link
Contributor Author

JohnCaccavale commented Jul 29, 2022

@mtrezza Just saw your message, I'm off from work today, let me know if there's anything I can do. I see there was a recently a PR merged with regards to increasing the time out of the carthage CircleCI timeout, I'm hoping that would help address the failure being shown here.

I pulled in your latest changes with regards to that fix that was merged. The tasks are currently running, will keep an eye on that status of them.

@mtrezza
Copy link
Member

mtrezza commented Jul 29, 2022

I just approved the workflow to run, let's see...

@parse-github-assistant
Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title fix: macOS command line app crash fix: MacOS command line app crash Jan 30, 2023
@mtrezza
Copy link
Member

mtrezza commented Jan 30, 2023

Carthage support has been removed, so if the CI passes, we can merge this finally

@JohnCaccavale
Copy link
Contributor Author

@mtrezza great! I won't have to keep applying my patch any longer 😂. Hopefully the tests will pass. 🤞

mtrezza
mtrezza previously approved these changes Jan 30, 2023
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Looks good! The failing macOS test is a flaky one.

@mtrezza mtrezza dismissed stale reviews from rodrigocarrapeiro and themself via 19b9ce1 January 30, 2023 10:58
@mtrezza mtrezza changed the title fix: MacOS command line app crash fix: macOS command line app crashes if Parse framework is installed in /Library/Frameworks/ Jan 30, 2023
@parse-github-assistant
Copy link

I will reformat the title to use the proper commit message syntax.

@parse-github-assistant parse-github-assistant bot changed the title fix: macOS command line app crashes if Parse framework is installed in /Library/Frameworks/ fix: MacOS command line app crashes if Parse framework is installed in /Library/Frameworks/ Jan 30, 2023
@mtrezza mtrezza merged commit 54bc6f3 into parse-community:master Jan 30, 2023
parseplatformorg pushed a commit that referenced this pull request Jan 30, 2023
## [2.0.2](2.0.1...2.0.2) (2023-01-30)

### Bug Fixes

* MacOS command line app crashes if Parse framework is installed in `/Library/Frameworks/` ([#1395](#1395)) ([54bc6f3](54bc6f3))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 2.0.2

@parseplatformorg parseplatformorg added the state:released Released as stable version label Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants