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

Don't pass a URL as a log destination to rust #2506

Merged
merged 6 commits into from
Feb 28, 2024

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented Feb 27, 2024

On macOS (catalyst) URL.appGroupContainerDirectory returns:

/Users/matthew/Library/Group%20Containers/group.io.element/

So you need to decode the %20 back into a space, otherwise logging fails with a permissions error.

Pull Request Checklist

On macOS (catalyst) URL.appGroupContainerDirectory returns:

/Users/matthew/Library/Group%20Containers/group.io.element/

So you need to strip the %20 out into a space, otherwise logging
fails with a permissions error.
@ara4n ara4n requested a review from a team as a code owner February 27, 2024 22:14
@ara4n ara4n requested review from pixlwave and removed request for a team February 27, 2024 22:14
Copy link

github-actions bot commented Feb 27, 2024

Warnings
⚠️ Some of the commits are missing ticket numbers. Please consider squashing all commits that don't have a tracking number.
⚠️ Please add a sign-off to either the PR description or to the commits themselves.

Generated by 🚫 Danger Swift against 6b7a304

Copy link

codecov bot commented Feb 27, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 74.69%. Comparing base (7615e97) to head (ed1db4f).

❗ Current head ed1db4f differs from pull request most recent head 6b7a304. Consider uploading reports for the commit 6b7a304 to get more accurate results

Files Patch % Lines
ElementX/Sources/Other/Logging/RustTracing.swift 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2506      +/-   ##
===========================================
+ Coverage    74.63%   74.69%   +0.05%     
===========================================
  Files          533      529       -4     
  Lines        37110    36974     -136     
  Branches     17679    17495     -184     
===========================================
- Hits         27697    27616      -81     
+ Misses        9196     9140      -56     
- Partials       217      218       +1     
Flag Coverage Δ
unittests 60.49% <50.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

LGTM, although would be nice to use path(percentEncoded:) as suggested.

ElementX/Sources/Other/Logging/RustTracing.swift Outdated Show resolved Hide resolved
@pixlwave pixlwave enabled auto-merge (squash) February 28, 2024 11:00
Copy link

sonarcloud bot commented Feb 28, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@stefanceriu stefanceriu merged commit 14d76d9 into develop Feb 28, 2024
5 checks passed
@stefanceriu stefanceriu deleted the matthew/fix-logging-on-macos branch February 28, 2024 12:28
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.

4 participants