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.

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

Ignore some paths from file watcher #3289

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

kshepherd
Copy link
Member

@kshepherd kshepherd commented Sep 3, 2024

Watching all these directories can cause systems to exceed maximum watched files / inotify limits, especially in dev mode with IDEs etc also watching files.

I've included node_modules, and some basic vcs and project dirs that should be excluded.

  watchOptions: {
    // Ignore directories that should not be watched for recompiling angular
    ignored: [
      '**/node_modules', '**/_build', '**/.git', '**/docker',
      '**/.angular', '**/.idea', '**/.vscode', '**/.history', '**/.vsix'
    ]
  },

Testing

It's actually not super trivial to 'see' watched files, but if you can reproduce the underlying issue (get your system into a state where inotify limit is exceeded), then you can run with and without this change, and review any errors listed, to see whether the file watcher is attempting to watch e.g. node_modules

(if anyone has any better test procedures please let me know!)

Notes

Documentation: https://webpack.js.org/configuration/watch/#watchoptions

Note, there is also a way to do this in tsconfig.json but in my practical testing it wasn't taking effect (see: https://www.typescriptlang.org/tsconfig/#watchOptions)

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • My PR is created against the main branch of code (unless it is a backport or is fixing an issue specific to an older branch).
  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using yarn lint
  • My PR includes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.

Watching all these directories can cause systems to exceed
maximum watched files / inotify limits, especially in dev
mode with IDEs etc also watching files.
@kshepherd kshepherd self-assigned this Sep 3, 2024
@tdonohue tdonohue added 1 APPROVAL pull request only requires a single approval to merge port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release labels Sep 3, 2024
@tdonohue
Copy link
Member

tdonohue commented Sep 3, 2024

Seems harmless to me, but I'd love to have a quick review from either @artlowel or @atarix83 .

The list of directories makes sense to me though! Thanks @kshepherd !

Copy link
Member

@artlowel artlowel left a comment

Choose a reason for hiding this comment

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

Thanks @kshepherd!

That makes sense

@tdonohue tdonohue merged commit d42d275 into DSpace:main Sep 4, 2024
13 checks passed
@tdonohue tdonohue added this to the 9.0 milestone Sep 4, 2024
@dspace-bot
Copy link
Contributor

Successfully created backport PR for dspace-7_x:

@dspace-bot
Copy link
Contributor

Successfully created backport PR for dspace-8_x:

@tdonohue tdonohue removed port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release labels Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 APPROVAL pull request only requires a single approval to merge
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants