Skip to content

Conversation

@mtopo27
Copy link
Contributor

@mtopo27 mtopo27 commented Oct 31, 2025

image

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@mtopo27 mtopo27 requested a review from a team as a code owner October 31, 2025 01:02
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Oct 31, 2025
mh: <InlineCode>__mh_execute_header</InlineCode>,
dlsym: <InlineCode>dlsym</InlineCode>,
})}
</Text>
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Bug

The tct call for adding symbols defines mh and dlsym in its interpolation object, but the template string only references [main]. This causes __mh_execute_header and dlsym to not be displayed, making the user instructions incomplete.

Fix in Cursor Fix in Web

@mtopo27 mtopo27 changed the title binary export insight chore(preprod): binary export insight Oct 31, 2025
<Flex direction="column" gap="md">
<Heading as="h3" size="md">
{t('Main Binary Export Metadata')}
</Heading>
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO delete this heading

in the screenshot it just shows the title twice which seems redundant

<Text>
{tct(
'[bold:How to fix]: Maintain a minimal allowlist so only required entry points stay exported.',
{bold: <strong />}
Copy link
Contributor

Choose a reason for hiding this comment

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

This modal UI seems inconsistent from the others

the other ones are

What it is

asdasd

How to fix

alsdkhaklsda

rather than

what it is: asdhashdlka
how to fix: asjkdaldjas

i think former is better

</li>
<li>
<Text>
{tct('Add [main] on its own line', {
Copy link
Contributor

@NicoHinderling NicoHinderling Oct 31, 2025

Choose a reason for hiding this comment

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

I feel like this line in the instructions is a bit too vague? maybe im dumb

</Text>
</li>
</ol>
<Text>{t('Xcode now limits the export trie to just that allowlist')}</Text>
Copy link
Contributor

Choose a reason for hiding this comment

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

u-nit: i think this phrasing sounds a bit better as Xcode will now limit the export trie to just that allowlist

but not a blocker

</Heading>
<Text>
{tct(
'[bold:What it is]: Binaries that act as entrypoints for your app, such as your main app binary or watchOS app binary, are not linked against by other binaries. This means the export trie information is unnecessary and can be removed.',
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can just delete the "what it is" text entirely from here

the title of the modal says what it is and the text right below it implicitly is the description of it

Copy link
Contributor

@NicoHinderling NicoHinderling left a comment

Choose a reason for hiding this comment

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

pre-stamp cus ❤️

@mtopo27 mtopo27 force-pushed the mtopo27/add-debug-symbol-insight branch from c827625 to 373fba4 Compare November 3, 2025 18:29
@mtopo27 mtopo27 force-pushed the mtopo27/add-binary-export-insight-modal branch from 870bf4c to f2f3c68 Compare November 5, 2025 03:00
Base automatically changed from mtopo27/add-debug-symbol-insight to master November 6, 2025 02:30
@mtopo27 mtopo27 force-pushed the mtopo27/add-binary-export-insight-modal branch from 9dc1eca to 6c45b3b Compare November 6, 2025 04:29
@mtopo27 mtopo27 merged commit d1da37a into master Nov 25, 2025
49 checks passed
@mtopo27 mtopo27 deleted the mtopo27/add-binary-export-insight-modal branch November 25, 2025 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants