search

minimistjs / minimist

parse argument options
MIT License
515 stars 30 forks source link

readme: add description of ECMAScript #62

Closed unikounio closed 4 months ago

unikounio commented 4 months ago

Added a description of ECMEScript to the readme and adjust the sample files to match.

Issue

Docs: Add sample code ES Modules format to the examples in the README. · Issue #61 · minimistjs/minimist

unikounio commented 4 months ago

@ljharb @shadowspawn Thank you both for your thorough reviews. I have made the necessary updates based on your feedback.

shadowspawn commented 4 months ago

Thanks for working through our various comments @unikounio

codecov-commenter commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.78%. Comparing base (6715df1) to head (98fe3c8).

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #62 +/- ## ======================================= Coverage 98.78% 98.78% ======================================= Files 1 1 Lines 165 165 Branches 70 70 ======================================= Hits 163 163 Misses 2 2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

unikounio commented 4 months ago

@ljharb Thank you for the thoughtful suggestion. I have addressed it.

shadowspawn commented 4 months ago

Urk. Failing a couple of lint tests due to using "new" features in the example code (const and modules). To be clear, these would be problems if they were in the actual implementation since we support very old node versions.

My first thought is examples using modern syntax are more appropriate and useful for new users. Is it appropriate to add an override to .eslintrc for the examples folder and allow modern features?

Do you already have a preferred way of approaching this @ljharb ?

ljharb commented 4 months ago

yep! add an "overrides" object for example/** in the root eslintrc, and extend @ljharb/eslint-config/node/latest.

shadowspawn commented 4 months ago

Are you happy to look at this @unikounio , or would you like a hand with the eslint change? (Fairly easy hopefully, but depending on what you already know and what you are interested in learning...)

unikounio commented 4 months ago

@shadowspawn Thank you for your suggestion. I would like to do this ESLint configuration change. Would it be better to include this change in this PR or would it be more appropriate to create a new PR for this change?

shadowspawn commented 4 months ago

Include in this PR thanks.

unikounio commented 4 months ago

Just pushed the changes to apply the latest ESLint config to the examples directory. Please take a look when you have a moment. Thanks!

unikounio commented 4 months ago

Thank you both so much for helping me with this PR! @ljharb and @shadowspawn, I deeply appreciate your support and the valuable feedback you've provided. I've learned a lot from this experience and am looking forward to any future opportunities to work together again!