-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Add missing semicolons for 'const' and 'let' #35540
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
Add missing semicolons for 'const' and 'let' #35540
Conversation
Ideally these files would conform to the coding standards.
Hi @fredden. Thank you for your contribution
❗ Automated tests can be triggered manually with an appropriate comment:
You can find more information about the builds here ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review. For more details, review the Magento Contributor Guide documentation. 🕙 You can find the schedule on the Magento Community Calendar page. 📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket. ✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
I guess it was partly fixed in faa7e0e |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔ Approved.
Failing tests look not related to changes from this PR.
@magento run Functional Tests B2B, Functional Tests CE , Functional Tests EE |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
Hi @fredden Our internal team informed me this issue has been fixed and committed . Hence, issue not reproducible to me in local 2.4-develop. As per your comment you have mentioned that this PR fixes other issue as well. Kindly provide me the information about the issue along with steps to reproduce, expected and actual result so, that we will be able to carry forward in our testing. |
@magento run Functional Tests EE |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
@magento run Functional Tests EE |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
@magento run Functional Tests EE |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time. |
Hi @fredden & @ihor-sviziev Thanks for your contribution and collaboration! |
@engcom-Lima this pull request was created in response to the error/issue that you have linked. The error message suggested that the built-in JavaScript minification was unable to support the To fix the problem, the JavaScript minification process could be updated to support this combination, or the source files could be updated to no longer trigger this incompatibility (or both). The Magento coding standard already requires semicolons in all cases, so the best way forward in my opinion would be to apply the Magento coding standard to the source files, which is what this pull request does. As noted in the pull request description, there are other violations of the Magento coding standard in these files, but I have opted to not fix those in scope of this pull request. |
@magento create issue |
If that is the case, how did this get in here in the first place? Don't we have linting/syntax testing in place for JS? This something worth raising a new issue for? |
Adobe staff do not follow the same workflow that is enforced for community contributions. |
Description
Semicolons are required according to the Magento coding standard:
https://github.com/magento/magento-coding-standard/blob/50e7c2ad3d7110b06527d75a1aa954b7c20f6ea1/eslint/.eslintrc-magento#L89
While there are many missing semicolons in these files, I have only fixed the
const
andlet
statements as these are causing problems for users in the admin - see #35325Ideally these files would conform to the Magento coding standard. Applying the auto-fix feature of
eslint
produced a much larger set of changes.Related Pull Requests
Fixed Issues
Manual testing scenarios
See #35325
Contribution checklist
Resolved issues: