Issue412

[salcio]

Fixes #412

Description of changes

Modified verifyTriggerWatchLocations to use glob.sync(trigger.watchPattern) instead of fs.readdirsync()

Checklist

If your change touches anything under src or the README.md file
these items must be done:

• [ x] User-facing change description added to unreleased section of CHANGELOG.md

[neilenns]

Thanks for the PR! A couple of general comments:

1. There's a build failure that needs to be resolved
2. Does this work with and without a glob pattern in watchFolder? I assume yes since you only changed dog, and not cat, detector in the sample.json file.

[salcio]

Thanks for the PR! A couple of general comments:

1. There's a build failure that needs to be resolved
2. Does this work with and without a glob pattern in watchFolder? I assume yes since you only changed dog, and not cat, detector in the sample.json file.

yes it works with and without the glob in pattern.

I'm working on the build failure (something to do with package-lock).

Edit:

Just passed build.

[neilenns]

Thanks for fixing the build failure! Looks like there are two build warnings now that need cleaning up in triggermanager.ts due to imports that are no longer used.

[neilenns]

Great thanks! I'll take another quick look at this tonight then merge it and produce a dev docker image for testing. Will run it locally a bit and ask you to try the dev build as well. If that all works out then I'll publish it as a release.

[salcio]

Thanks for fixing the build failure! Looks like there are two build warnings now that need cleaning up in triggermanager.ts due to imports that are no longer used.

Yeah you are right. I haven't seen the warnings. Now fixed.
You might want to consider settings lint to cause build failure when warnings like that happen - will save time :).

[neilenns]

Thanks for fixing the build failure! Looks like there are two build warnings now that need cleaning up in triggermanager.ts due to imports that are no longer used.

Yeah you are right. I haven't seen the warnings. Now fixed.
You might want to consider settings lint to cause build failure when warnings like that happen - will save time :).

Solid suggestion. Feel free to open a bug and a PR to get that change in 😂 At the very least open an issue to track it so I can get to it next time I'm touching code.

[salcio]

Thanks for fixing the build failure! Looks like there are two build warnings now that need cleaning up in triggermanager.ts due to imports that are no longer used.

Yeah you are right. I haven't seen the warnings. Now fixed.
You might want to consider settings lint to cause build failure when warnings like that happen - will save time :).

Solid suggestion. Feel free to open a bug and a PR to get that change in 😂 At the very least open an issue to track it so I can get to it next time I'm touching code.

Make sense. Issue created #414

[neilenns]

Code looks good. I'll re-work the text in the release notes when I push the release out.

One requested change: instead of aiinput/asdf as the sub-folder please change to a real folder name that matches what's inside. I suggest aiinput/dogcamera or similar.

[salcio]

Code looks good. I'll re-work the text in the release notes when I push the release out.

One requested change: instead of aiinput/asdf as the sub-folder please change to a real folder name that matches what's inside. I suggest aiinput/dogcamera or similar.

yeah I should have pay more attention before submitting PR. Done some testing and then you see :).
It's changed now.

[salcio]

@salcio Heads up it looks like you merged another branch into this one.

Note that if you are planning to open a future pull request to archive the input files I'm unlikely to take it. I don't believe this system should be responsible for managing files it didn't create.

uuu I didn't want to do that. I wanted to merge the other way. I'll revert that merge.

Makes sense with not touching the files that were created by someone else. I didn't plan to create PR for this as it was more for my setup (ease of adding functionality to existing app rather then creating completely new one)