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

chore: fix javascript lint errors #6214 #6221

Open
wants to merge 30 commits into
base: develop
Choose a base branch
from
Open

chore: fix javascript lint errors #6214 #6221

wants to merge 30 commits into from

Conversation

bhavishy2801
Copy link
Contributor

@bhavishy2801 bhavishy2801 commented Mar 20, 2025 •
edited
Loading

Resolves #6214

Description

What is the purpose of this pull request?

This pull request:

  • Fixes the linting failures were detected in the automated JavaScript lint workflow run.

Related Issues

Does this pull request have any related issues?

This pull request:

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

No.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

@stdlib-js/reviewers

@stdlib-bot stdlib-bot added Needs Review A pull request which needs code review. Good First PR A pull request resolving a Good First Issue. labels Mar 20, 2025
@stdlib-bot
Copy link
Contributor

stdlib-bot commented Mar 20, 2025 •
edited
Loading

Coverage Report

Package Statements Branches Functions Lines
proxy/ctor $\color{green}174/174$
$\color{green}+100.00\%$ 		$\color{red}6/7$
$\color{green}+85.71\%$ 		$\color{green}1/1$
$\color{green}+100.00\%$ 		$\color{green}174/174$
$\color{green}+100.00\%$

The above coverage report was generated for the changes in this PR.

@bhavishy2801 bhavishy2801 changed the title Fix java script lint errors #6214 chore: fix javascript lint errors #6214 Mar 20, 2025
kgryte
kgryte reviewed Mar 21, 2025
View reviewed changes
lib/node_modules/@stdlib/proxy/ctor/lib/main.js Outdated
Copy link
Member

Choose a reason for hiding this comment

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

The changes to this file should be reverted, as they are not desired. The entire point of the doctest is to demonstrate how proxies allow for intercepting property access.

kgryte
kgryte reviewed Mar 21, 2025
View reviewed changes
lib/node_modules/@stdlib/proxy/ctor/lib/polyfill.js
Copy link
Member

Choose a reason for hiding this comment

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

The proposed changes are not the desired changes, in this case. Instead, because we still need the polyfill implementation, at L47, you should just disable doctesting using a eslint-disable-line comment. This rule can be re-enabled once the polyfill is completed.

Copy link
Member

Choose a reason for hiding this comment

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

You have also yet to revert the changes to this file.

@kgryte kgryte added Needs Changes Pull request which needs changes before being merged. and removed Needs Review A pull request which needs code review. labels Mar 21, 2025
@bhavishy2801
Update polyfill.js
701c8b6
@bhavishy2801
Update polyfill.js
7e15762
@bhavishy2801
Updated polyfill.js
b2d2c7c 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
b163856 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
e7dccbd 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
ed0570c 
Signed-off-by: Bhavishy Agrawal <[email protected]>
kgryte
kgryte reviewed Mar 23, 2025
View reviewed changes
lib/node_modules/@stdlib/proxy/ctor/lib/polyfill.js Outdated
Comment on lines 47 to 48
function Proxy(target) {
// eslint-disable-next-line no-warning-comments
Copy link
Member

Choose a reason for hiding this comment

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

Still incorrect. And you shouldn't have changed the file indentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Got the fix. Working on that now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kgryte please review

@bhavishy2801
Updated polyfill.js
df9cd46 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
cc54540 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
175b6c8 
Signed-off-by: Bhavishy Agrawal <[email protected]>
kgryte
kgryte reviewed Mar 23, 2025
View reviewed changes
lib/node_modules/@stdlib/proxy/ctor/lib/polyfill.js
*/
function Proxy( target ) {
// eslint-disable-next-line no-warning-comments
Copy link
Member

Choose a reason for hiding this comment

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

Still incorrect.

kgryte
kgryte reviewed Mar 23, 2025
View reviewed changes
lib/node_modules/@stdlib/proxy/ctor/lib/polyfill.js Outdated
@@ -30,7 +30,7 @@
*
* @example
* function get( obj, prop ) {
* return obj[ prop ] * 2.0;
* return obj[ prop ] * 1.0;
Copy link
Member

Choose a reason for hiding this comment

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

This change should be reverted.

kgryte
kgryte reviewed Mar 23, 2025
View reviewed changes
lib/node_modules/@stdlib/proxy/ctor/lib/polyfill.js Outdated
@@ -42,9 +42,10 @@
* p.a = 3.14;
*
* var x = p.a;
* // returns 6.28
* // returns 3.14
Copy link
Member

Choose a reason for hiding this comment

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

This change should be reverted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kgryte Done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kgryte but now, it's failing the check in lint_changed_files,

`Run # Determine root directory:
make[1]: Entering directory '/home/runner/work/stdlib/stdlib'

Linting file: lib/node_modules/@stdlib/proxy/ctor/lib/polyfill.js

/home/runner/work/stdlib/stdlib/lib/node_modules/@stdlib/proxy/ctor/lib/polyfill.js
Error: 23:1 error Displayed return value is 6.28, but expected 3.14 instead stdlib/jsdoc-doctest

✖ 1 problem (1 error, 0 warnings)
1 error and 0 warnings potentially fixable with the --fix option.

make[1]: *** [/home/runner/work/stdlib/stdlib/tools/make/lib/lint/javascript/eslint.mk:249: eslint-files] Error 1
make[1]: Leaving directory '/home/runner/work/stdlib/stdlib'
make: *** [/home/runner/work/stdlib/stdlib/tools/make/lib/lint/javascript/Makefile:158: lint-javascript-files] Error 2
Error: Process completed with exit code 2.`

Copy link
Member

Choose a reason for hiding this comment

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

Yes, precisely, and that error message tells you exactly the lint rule you need to disable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kgryte I've made some changes in the code of polyfill.js, but still it's showing the error:-

Error: 47:1 error Encountered an orphaned return annotation without a preceding node stdlib/jsdoc-doctest-marker

If I remove the * // eslint-disable-next-line stdlib/jsdoc-doctest-marker line, it will show error to use 3.14 instead of 6.28, and if I use the line, it shows the above error. Please guide

@bhavishy2801
Updated polyfill.js
43e1bbc 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated main.js
4d5dfa3 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
09f7346 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
6980d23 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
5b12782 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
3b71ac6 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
f1e2e47 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
135d208 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
d6d8015 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
5f189c0 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
51620f6 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
a092b94 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
7e5a0f6 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
b43e355 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
a7c39ab 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
da25f36 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
978501e 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
b8c4077 
Signed-off-by: Bhavishy Agrawal <[email protected]>
@bhavishy2801
Updated polyfill.js
a90e1e3 
Signed-off-by: Bhavishy Agrawal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kgryte kgryte kgryte left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Good First PR A pull request resolving a Good First Issue. Needs Changes Pull request which needs changes before being merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix JavaScript lint errors
3 participants
@bhavishy2801 @stdlib-bot @kgryte