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

Refactor FeatureDefinitionContext to extend ContextStore #964

Merged
merged 1 commit into from
Apr 15, 2022

Conversation

KingOfSquares
Copy link
Contributor

In preparation for finalizing #950 I decided to split out this part into its own PR.

The idea behind the change is to be able to depend on the general ContextStore in places where you need some FeatureDefinition that may be in its own context on legacy maps(proto 1.3), e.g FilterContext or in the general FeatureDefinitionContext on newer maps(1.4 proto). Then the correct context can be passed without having to instance check for different types of contexts or other hackery.

I also decided to update naming in the ContextStore, as it is now used in newer protos where id is the proper term.(in contrast to name)

Also other small changes...

Note: The Map used to store feature definitions is now a TreeMap(Same as ContextStore has been using until now), which technically could reduce performance but its so small that I think its negligible.

KingOfSquares added a commit to KingOfSquares/PGM that referenced this pull request Feb 7, 2022
Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>
Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>
@KingOfSquares
Copy link
Contributor Author

In an attempt to move this faster the PR has been slimmed down drastically to only include necessary changes for the code to work

@Pablete1234 Pablete1234 added the ready PR is ready to merge label Apr 12, 2022
Copy link
Member

@Electroid Electroid left a comment

Choose a reason for hiding this comment

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

Looks good.

@Electroid Electroid enabled auto-merge (squash) April 15, 2022 16:15
@Electroid Electroid disabled auto-merge April 15, 2022 16:15
@Electroid Electroid merged commit e2adc6e into PGMDev:dev Apr 15, 2022
KingOfSquares added a commit to KingOfSquares/PGM that referenced this pull request Apr 18, 2022
Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>
KingOfSquares added a commit to KingOfSquares/PGM that referenced this pull request Apr 18, 2022
Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>
KingOfSquares added a commit to KingOfSquares/PGM that referenced this pull request Apr 22, 2022
Signed-off-by: KingSimon <19822231+KingOfSquares@users.noreply.github.com>
KingOfSquares added a commit to KingOfSquares/PGM that referenced this pull request May 19, 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>
KingOfSquares added a commit to KingOfSquares/PGM that referenced this pull request Jun 17, 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>
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.

4 participants