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.

Sign up for GitHub

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

Jump to bottom

Use explicit imports in our javascript source files #36854

Merged
merged 5 commits into from
Oct 26, 2022
Merged

Use explicit imports in our javascript source files #36854

merged 5 commits into from
Oct 26, 2022

Conversation

GeoSot
Copy link
Member

@GeoSot GeoSot commented Jul 27, 2022 •
edited
Loading

closes #36665

ref: #31944

@GeoSot GeoSot mentioned this pull request Aug 6, 2022
Open
@GeoSot GeoSot force-pushed the gs/add-explicit-paths-to-js-files branch from bd40086 to dfdbd92 Compare September 2, 2022 14:20
@mdo mdo mentioned this pull request Sep 2, 2022
Closed
3 tasks
@GeoSot GeoSot force-pushed the gs/add-explicit-paths-to-js-files branch from dfdbd92 to fd36c39 Compare September 18, 2022 17:40
@GeoSot GeoSot force-pushed the gs/add-explicit-paths-to-js-files branch from fd36c39 to bfeb348 Compare October 3, 2022 18:08
@GeoSot GeoSot marked this pull request as ready for review October 5, 2022 14:10
@GeoSot GeoSot requested a review from a team as a code owner October 5, 2022 14:10
@GeoSot GeoSot force-pushed the gs/add-explicit-paths-to-js-files branch from c6dee54 to d9db204 Compare October 7, 2022 21:26
julien-deramond
julien-deramond approved these changes Oct 14, 2022
View reviewed changes
Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

Just added a small comment regarding a line-break.

I didn't manage to reproduce locally exactly the same behavior as what's described in the issue but I understand what's the issue and this PR corresponds well to what's expected so LGTM!

@XhmikosR You have more experience than me regarding this kind of topic so if you want to check this PR as well :)

build/build-plugins.js Outdated Show resolved Hide resolved
@XhmikosR
Copy link
Member

Yeah, this is something xo kept warning about in my xo branch, but never got to fix it myself. Thanks @GeoSot!

@XhmikosR
Copy link
Member

Has anyone tested this and confirmed it fixes the problems people mentioned? Just quickly reviewing some PRs here.

@julien-deramond
Copy link
Member

Has anyone tested this and confirmed it fixes the problems people mentioned? Just quickly reviewing some PRs here.

On my side I didn't manage to reproduce locally exactly the same behavior as what's described in the issue so I can't confirm with certitude that it fixes the issue. I think it does but not 100% certain.

@XhmikosR
Copy link
Member

Let's try it I guess since it doesn't look like it can cause any breakage.

@XhmikosR XhmikosR merged commit aa9d32d into main Oct 26, 2022
@XhmikosR XhmikosR deleted the gs/add-explicit-paths-to-js-files branch October 26, 2022 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julien-deramond julien-deramond julien-deramond approved these changes

@XhmikosR XhmikosR XhmikosR approved these changes

Assignees
No one assigned
Labels
build js release-highlight squash v5
Projects
No open projects
v5.3.0
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

use explicit path with imports (add .js)
3 participants
@GeoSot @XhmikosR @julien-deramond