Skip to content

fix(xcode): Improve Xcode error msg when config load fails#2028

Merged
elramen merged 4 commits intomasterfrom
elramen/fix-xcode-config-error-msg
Apr 11, 2024
Merged

fix(xcode): Improve Xcode error msg when config load fails#2028
elramen merged 4 commits intomasterfrom
elramen/fix-xcode-config-error-msg

Conversation

@elramen
Copy link
Copy Markdown
Member

@elramen elramen commented Apr 11, 2024

When a user sets up a project in xcode, e.g., by using the wizard, the .sentryclirc config is created in the project folder. However, when this file isn't included in the Input Files of the Xcode script for uploading debug files, an error is thrown in Xcode which isn't very clear. This commit improves that error message by specifying what the user needs to do.

Fixes GH-1924

When a user sets up a project in xcode, e.g., by using the wizard, the .sentryclirc config is created in the project folder. However, when this file isn't included in the Input Files of the Xcode script for uploading debug files, this causes an error in Xcode which is not very clear. This commit improves that error message.
@elramen elramen requested a review from szokeasaurusrex April 11, 2024 11:55
Copy link
Copy Markdown
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

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

Two small nitpicks – you can choose whether or not to implement the changes – this code looks pretty good though! Great first PR!

src/config.rs Outdated
let msg = format!("Failed to load {file_desc}.");
#[cfg(target_os = "macos")]
if xcode::launched_from_xcode() {
return msg + " Hint: Please ensure that ${SRCROOT}/.sentryclirc is added to the Input Files of this Xcode Build Phases script.";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also nitpick: I slightly preferred your previous implementation with push_str here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@szokeasaurusrex Clippy would give an error for having msg as mutable

Copy link
Copy Markdown
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

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

Code looks good!

One last thing before merging, however: please make sure the PR title and comment follow the Sentry commit naming conventions. I specifically would recommend changing the title to "fix(xcode)" (lowercase). Please also make sure to reference the issue that this PR addresses at the end of the PR description, like so:

Fixes GH-<issue number here>

@elramen elramen changed the title Fix: Improve Xcode error msg when config load fails fix(xcode): Improve Xcode error msg when config load fails Apr 11, 2024
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.

Update file permissions error message and docs

2 participants