-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
chore: fix JavaScript lint errors #8590
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: fix JavaScript lint errors #8590
Conversation
|
Hello! Thank you for your contribution to stdlib. We noticed that the contributing guidelines acknowledgment is missing from your pull request. Here's what you need to do:
This acknowledgment confirms that you've read the guidelines, which include:
We can't review or accept contributions without this acknowledgment. Thank you for your understanding and cooperation. We look forward to reviewing your contribution! |
kgryte
left a comment
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.
Thanks for working on this, @Amansingh0807.
I think my main concern with the proposed changes is that we assume that the examples work correctly and we don't need to perform any clean-up. E.g., after creating a temporary file, we could encounter an error before unlinking, resulting in the temporary file hanging around and potentially getting committed.
Alternatively, we could use @stdlib/os/tmpdir to write to the host's temporary directory.
We're also making certain assumptions about this particular file's location relative to the examples directory. E.g., suppose we were to reorganize the examples folder and move the fixtures folder somewhere else. How would a developer know that they inadvertently broke examples in the lib folder? The answer is that they probably wouldn't.
So in short, in their current form, I don't think we can accept the proposed changes. I suggest reworking the examples to be more robust.
Thank you for the detailed feedback, @kgryte! I've updated the examples to address your concerns: Changes:
Please let me know if any further adjustments are needed. |
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.
Thanks for addressing the feedback!
|
@Planeshifter I am not actually convinced these changes are necessary. |
|
The examples could simply be updated such that, instead of throwing, we do |
|
As the examples for this tool are meant to be illustrative, IMO creating temporary files and cleaning up seems like overkill. |
|
@kgryte Yeah, that's fair. So let's instead move away from throwing errors in such example code involving files and instead use the Thank you @Amansingh0807 for your efforts and contributions to |
Resolves #8586
Description
This pull request:
insert-header-file-listmodule that were causing the "Lint JavaScript files" workflow to fail with an ENOENT error.Related Issues
This pull request has the following related issues:
Questions
No.
Other
No.
Problem
The JSDoc
@examplecode blocks inlib/node_modules/@stdlib/_tools/licenses/insert-header-file-list/lib/index.jsreferenced a non-existent file path'./foo/bar.js'. Since stdlib's customjsdoc-doctestESLint rule actually executes JSDoc examples in a VM to verify they work correctly, it attempted to run theinsertHeader()function with this non-existent file, resulting in - Error: ENOENT: no such file or directory, open '/home/runner/work/stdlib/stdlib/foo/bar.js'Solution
Updated both JSDoc examples to use realistic, executable code that:
examples/fixtures/file.js.txt)insertHeader()function on the temporary fileThis pattern now matches the working example already present in
examples/index.jsand ensures thejsdoc-doctestrule can execute successfully without errors.Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
@stdlib-js/reviewers