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

Draft pull request Branch: feature/alarms+anomaly detection #1222

Draft
wants to merge 176 commits into
base: master
Choose a base branch
from

Conversation

PepijnMuskens
Copy link
Collaborator

Still work in progress but has the general anomaly detection functionality working with 3 detection methods.

Also includes Nina's work from the alarms up until dec. 22 2023

NinaBianca and others added 5 commits January 22, 2024 17:16
- Finish notification sending
- Add asset to table for wide screen
- Multiselect delete (solve occasional select-all glitch)
@MartinaeyNL
Copy link
Contributor

MartinaeyNL commented Feb 2, 2024

Note (to self):

  • Code is not fully finished; still needs cleanup. Worth some code analysis like SonarLint.
  • This PR partly includes code from Nina regarding the alarms; ignore this.
  • Tests have not been written yet; should be there before merging.
  • Be aware of small changes for local use such as the CI/CD file containing OR_SETUP_RUN_ON_RESTART set to false.
    Needs to be cleaned up before merge.

Almost done with a 1st review, will post start of next week.

@MartinaeyNL
Copy link
Contributor

MartinaeyNL commented Feb 5, 2024

To add some additional notes about the review;

The code was looking good overall! Most of the bits I commented on are code quality related,
simply to maintain reusability or readability. For example in regards to method separation / single responsibility of functions,
or "code cleanup bits" you only spot during reviews like these haha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants