Update quickstart.md, added new 4th step making it 5 step documentation

[Dev-Arhaan]

Fixes

Fixes #2616 by @dhruvkb

Description

Added pnpm filtering command to restrict commands to @openverse/eslint-plugin as new 4th step.

Testing Instructions

Checklist

• My pull request has a descriptive title (not a vague title likeUpdate index.md).
• My pull request targets the default branch of the repository (main) or a parent feature branch.
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added or updated tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no visible errors.
• I ran the DAG documentation generator (if applicable).

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[AetherUnbound]

Thanks for your contribution! In addition to Krystle's correction, I have one additional suggestion.

[Dev-Arhaan]

Your welcome! Hoping to contribute more to this project in future. Tell me if there is any other thing I can do

[sarayourfriend]

@dhruvkb and @AetherUnbound @krysal who have reviewed. I think #2633 is a better solution for this because it removes this unnecessary dependency. Alternatively, we should add building the ESLint package to postinstall in the frontend package, if we really want to keep the ESLint Webpack warnings plugin.

That is to say, additional instructions are not the solution here because we can easily automate the building of the package in our regular process. Which is far better, because then the plugin will actually be updated when it changes and is evaluated by the Webpack plugin.

[Dev-Arhaan]

This can be merged now? review it and let me know if I can improve anything else

[AetherUnbound]

@sarayourfriend thanks for your perspective, I was primarily reviewing based on the context from the issue (since I don't work to directly with these tools). Do you think this is sufficiently addressed by #2643?

@Dev-Arhaan thanks for your patience while we figure out the best path forward on this!

[dhruvkb]

While #2633 does fix the case where ESLint blocks the dev window, it doesn't address the fact that the ESLint package is not compiled when the dev server is run directly. I would prefer this to be in the docs.

[dhruvkb]

This seems an unnecessary repetition of the previous command. We could omit this.

[sarayourfriend]

While #2633 does fix the case where ESLint blocks the dev window, it doesn't address the fact that the ESLint package is not compiled when the dev server is run directly. I would prefer this to be in the docs.

The eslint plugin is only a dependency of the frontend because of the ESLint Webpack plugin. The frontend Nuxt build shouldn't need the ESLint plugin at all and therefore there's no need to manually run a build of it. When the package.json eslint script runs, it does build the plugin so that it's available when it is needed.

Is there something else that is causing a dependency between the built @openverse/eslint-plugin package and dev? There shouldn't be 🤔

If it were, for some reason, then the correct way to fix this would be to add build --watch for the plugin to the frontend dev script so that it automatically happens and self-updates when changes happen to the code.

The note at the end of this page describes this apporoach: https://docs.openverse.org/packages/#adding-new-packages

[openverse-bot]

Based on the high urgency of this PR, the following reviewers are being gently reminded to review this PR:

@sarayourfriend
This reminder is being automatically generated due to the urgency configuration.

Excluding weekend¹ days, this PR was ready for review 3 day(s) ago. PRs labelled with high urgency are expected to be reviewed within 2 weekday(s)².

@Dev-Arhaan, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings.

Footnotes

1. Specifically, Saturday and Sunday. ↩

2. For the purpose of these reminders we treat Monday - Friday as weekdays. Please note that the operation that generates these reminders runs at midnight UTC on Monday - Friday. This means that depending on your timezone, you may be pinged outside of the expected range. ↩

[sarayourfriend]

I'll draft this until @dhruvkb responds to prevent further unnecessary pings.

[dhruvkb]

Sorry, it took me a while to process your message @sarayourfriend. I think IDEs like WebStorm run ESLint constantly in the background to underline issues. They will break unless the extension is compiled. I would prefer an approach that has the package compiled even before running dev, so that my IDE doesn't complain, hence documenting the step seemed like the right approach.

[sarayourfriend]

I would prefer an approach that has the package compiled even before running dev, so that my IDE doesn't complain, hence documenting the step seemed like the right approach.

Gotcha. I hadn't considered that aspect of devex. Could we add it as a postinstall step in the root package.json? The main thing I want to avoid is adding yet another step for setup if there's any way at all that we can automatically handle it.

I suggest postinstall because ESLint won't work correctly at all without pnpm install anyway.

What do you think?

[dhruvkb]

@sarayourfriend that seems like a good idea. Could you please update the issue to reflect this change of approach and also add some steps for how to resolve it?

@Dev-Arhaan thanks for working on this and we're sorry for not being able to utilise this PR. Once, the issue has been updated to use postinstall, would you be open to making a PR using that approach?

[sarayourfriend]

Done 👍

[Dev-Arhaan]

@sarayourfriend that seems like a good idea. Could you please update the issue to reflect this change of approach and also add some steps for how to resolve it?

@Dev-Arhaan thanks for working on this and we're sorry for not being able to utilise this PR. Once, the issue has been updated to use postinstall, would you be open to making a PR using that approach?

Hey, I saw this today, I was/am busy with moving to hostel, just let me know what needs to be added and I'll put a pr out by day after tomorrow

[krysal]

We appreciate your help here @Dev-Arhaan. Please review the updated issue and make another PR to continue.