Skip to content

Conversation

musvaage
Copy link

Closes #7531

using master for this pull

Copy link
Contributor

@camdecoster camdecoster left a comment

Choose a reason for hiding this comment

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

I made a few comments about how to proceed. Unfortunately, a lot of these typos are from dependencies and won't be able to be fixed inside plotly.js.

Copy link
Contributor

Choose a reason for hiding this comment

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

This change likely altered the result of some test images, which makes sense. You can download the new images from the Artifacts tab in the CircleCI results and save those into the test images folder and commit them.

Copy link
Author

@musvaage musvaage Aug 25, 2025

Choose a reason for hiding this comment

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

download the new images from the Artifacts tab in the CircleCI results

I'm unclear on how to do that.

test images folder

I'm unclear on the precise location of the "test images" folder.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll take care of it. It can be confusing if you're unfamiliar with the testing system.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you allow edits to the PR by maintainers?

Copy link
Author

Choose a reason for hiding this comment

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

Allow edits from maintainers

I don't see that text and documentation on that invariably refer to a fork's user created branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should show up on the PR as described here. If you aren't able to do that, I'll just create my own PR that incorporates your changes.

Copy link
Author

Choose a reason for hiding this comment

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

aren't able to do that

"you can modify an existing pull request to let repository maintainers make commits to your branch"

I used the master branch for this pull, maybe that's why the Allow edits... text doesn't appear.

I'll just create my own PR that incorporates your changes.

That seems to be the solution to this circumstance.

@musvaage
Copy link
Author

musvaage commented Aug 25, 2025

virtual-webgl-jasmine - Failed

Cannot start Chrome

@musvaage
Copy link
Author

Rather than elaborate on how to get the checks to pass, someone else familiar with that could open a successor PR.

The sed lines with the typo fixes appear in the linked to ticket, but then maybe the fixes are not really worth the effort.

@gvwilson gvwilson added community community contribution P2 considered for next cycle fix fixes something broken labels Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community community contribution fix fixes something broken P2 considered for next cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

typos
3 participants