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

Introduce Monostable filters #950

Merged
merged 3 commits into from
Jul 9, 2022

Conversation

KingOfSquares
Copy link
Contributor

Allows for time period based filters.
This PR also makes the existing time filter dynamic.

I would like feedback on the design, old PGM used the filter definitions as FeatureFactorys to create the Reactor instances, but that is not ideal for this PGM because it does not use injection.

Signed-off-by: KingSimon 19822231+KingOfSquares@users.noreply.github.com

@KingOfSquares
Copy link
Contributor Author

Seems to have failed since electroids repo is down

@KingOfSquares
Copy link
Contributor Author

Ive drafted changes based on the feedback. I have yet to test these changes, so don't merge them yet 🙃

@KingOfSquares
Copy link
Contributor Author

KingOfSquares commented Feb 4, 2022

Currently waiting for #964
EDIT: Not anymore! Ready for review

@KingOfSquares KingOfSquares force-pushed the dynamic-time-filter branch 2 times, most recently from ea834bc to 810afbb Compare April 18, 2022 22:23
@KingOfSquares
Copy link
Contributor Author

I've pushed the response to code review for initial review again, but the code is not yet tested on a server

@KingOfSquares
Copy link
Contributor Author

Ok code has been tested an the changes seem to work as expected

Copy link
Member

@Pablete1234 Pablete1234 left a comment

Choose a reason for hiding this comment

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

just a couple small changes, otherwise lgtm

Pablete1234
Pablete1234 previously approved these changes Apr 22, 2022
Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>
format :(

Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>

Feedback changes

Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>

Changes from feedback and draft for PGMDev#964

Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>

Move null check

Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>

finish initial implementation

Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>

javadoc changes

Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>

format

Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>

review feedback

Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>

more review feedback

Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>
Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>
Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>
@Pablete1234 Pablete1234 added the ready PR is ready to merge label Jun 17, 2022
@Pablete1234 Pablete1234 mentioned this pull request Jul 1, 2022
20 tasks
@Electroid Electroid merged commit 312f6da into PGMDev:dev Jul 9, 2022
@KingOfSquares KingOfSquares deleted the dynamic-time-filter branch July 12, 2022 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready PR is ready to merge
Development

Successfully merging this pull request may close these issues.

3 participants