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

chore: improve preload comment #1552

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

yangannyx
Copy link
Member

Simple text change to the comment at the top of the the default preload.js file. Clarifies what the preload script runs before.

Screenshot 2024-02-23 at 12 16 11 PM

@yangannyx yangannyx requested review from codebytere and a team as code owners February 23, 2024 20:23
@coveralls
Copy link

Coverage Status

coverage: 87.245%. remained the same
when pulling b8c52a1 on yangannyx:anny/better-preload-comments
into 4bf7ed9 on electron:main.

Copy link
Member

@dsanders11 dsanders11 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Agreed the current wording is less than ideal.

The content in Fiddle actually gets loaded from electron/electron-quick-start by looking for a branch there (e.g. 29-x-y) and using the content there first. The file you've touched in Fiddle is the fallback if the other is not found.

As such, let's first make this change in that repo so that these don't diverge, and then we can land it here.

See https://github.com/electron/electron-quick-start/blob/main/preload.js

@yangannyx
Copy link
Member Author

Thanks for the pointer! Made the same change to electron/electron-quick-start here

@dsanders11 dsanders11 changed the title fix: improve preload comment chore: improve preload comment Feb 26, 2024
Copy link
Member

@dsanders11 dsanders11 left a comment

Choose a reason for hiding this comment

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

Changed the title to be chore: rather than fix: as I think that's more accurate in this case.

@dsanders11 dsanders11 merged commit 8b9c116 into electron:main Feb 26, 2024
11 checks passed
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.

3 participants