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

docs: Fix jazzy undocumented symbols #1820

Closed
wants to merge 7 commits into from

Conversation

dplewis
Copy link
Member

@dplewis dplewis commented Oct 30, 2024

New Pull Request Checklist

Issue Description

There are a lot of errors when generating docs that prevents symbols from being documented.
https://github.com/parse-community/Parse-SDK-iOS-OSX/actions/runs/11596782336/job/32288646411

error: 'BFTask.h' file not found
error: module 'Bolts' not found

Before
Screenshot 2024-10-30 at 3 02 03 PM

After
Screenshot 2024-10-30 at 4 45 43 PM

Approach

  • Move @import Bolts to .m and add @class to .h
  • Remove duplicate properties like dataAvailable from catetories
  • Add missing imports for Constants
  • Properly handle Bolts import
  • Remove old carthage/pods imports
  • Remove console errors

TODOs before merging

  • Add changes to documentation (guides, repository pages, in-code descriptions)

Copy link

parse-github-assistant bot commented Oct 30, 2024

Thanks for opening this pull request!

  • ❌ Please link an issue that describes the reason for this pull request, otherwise your pull request will be closed. Make sure to write it as Closes: #123 in the PR description, so I can recognize it.

@dplewis dplewis requested a review from a team October 30, 2024 23:06
Copy link

codecov bot commented Oct 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.79%. Comparing base (dd05d41) to head (ac4eafd).
Report is 29 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1820       +/-   ##
===========================================
+ Coverage   64.24%   82.79%   +18.55%     
===========================================
  Files         201      282       +81     
  Lines       23233    30726     +7493     
===========================================
+ Hits        14926    25441    +10515     
+ Misses       8307     5285     -3022     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Why are the tests failing

@dplewis
Copy link
Member Author

dplewis commented Oct 31, 2024

@mtrezza For some reason functions that return BFTask can't be called in swift after this change.

+ (nonnull BFTask<NSNumber *> *)trackEvent:(nonnull NSString *)name;
class func trackEvent(_ name: String) -> BFTask

@mtrezza
Copy link
Member

mtrezza commented Oct 31, 2024

I also wanted to say that there a quite a few changes in code so not sure if that can be considered "only " a docs PR?

@dplewis
Copy link
Member Author

dplewis commented Nov 8, 2024

@mtrezza I found a cleaner way to do this, simple refactoring. I think the build is broken too

@mtrezza
Copy link
Member

mtrezza commented Nov 8, 2024

The build worked in the PR last week, so I assume it's this PR that breaks it?

@dplewis
Copy link
Member Author

dplewis commented Nov 8, 2024

All my PR’s have flaky test introduced, simulators we use aren’t available anymore in certain version of Xcode, I don’t know if it’s a github action issue

@mtrezza
Copy link
Member

mtrezza commented Nov 9, 2024

Not sure I understand; the CI currently passes, see for example https://github.com/parse-community/Parse-SDK-iOS-OSX/actions/runs/11758150963. All jobs passed at first run, but to be sure, I've restarted the CI for another run to see if there is any flakiness. I've restarted the CI here too, so we can compare the errors between the two runs.

@mtrezza
Copy link
Member

mtrezza commented Nov 11, 2024

Turns out the CI is also flaky in that other PR. So I'll just rerun the CI here until hopefully all pass.

@dplewis dplewis closed this Nov 14, 2024
@mtrezza
Copy link
Member

mtrezza commented Nov 16, 2024

@dplewis I couldn't get the CI to pass, even after re-running the failed tests multiple times. What should we do here?

@dplewis
Copy link
Member Author

dplewis commented Nov 16, 2024

I closed this because it introduces a breaking change when using this SDK in a swift environment. Can you do a release PR? The Xcode 16 support got added. If that PR has flaky test then I don’t know what to do but as least we can investigate

@mtrezza
Copy link
Member

mtrezza commented Nov 16, 2024

Are the changes in 0be6870 necessary to use the SDK in Xcode 16? I assumed it only added the Xcode 16 tests to the CI and the gemfile dependency upgrades were done additionally but not required for Xcode 16. Hence this wasn't merged as feature release.

@dplewis
Copy link
Member Author

dplewis commented Nov 17, 2024

Just a suggestion if you want to update the change log then a release would be nice. A patch release would work as no functionality has changed.

@mtrezza
Copy link
Member

mtrezza commented Nov 18, 2024

I think you are right, that should have been merged as feat, not as ci. Let me see how I can trigger a release here. Opened #1823.

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