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

gh-2552-remove-deprecated MultiSerialiser #2587

Merged

Conversation

GCHQDev404
Copy link
Contributor

@GCHQDev404 GCHQDev404 commented Feb 14, 2022

update the MultiSerialiser wanting to emphasise the effectiveness of it during the deprecation of serialisers.
8 is the last and best.
24 is not latest regardless of natural-order.
7 is dead.

Related Issue

* },
* "valueClass" : "java.lang.Integer"
* }, {
* "key" : 8,
* "serialiser" : {
* "class" : "uk.gov.gchq.gaffer.serialisation.implementation.ordered.OrderedIntegerSerialiser"
* "class" : "uk.gov.gchq.gaffer.serialisation.implementation.raw.CompactRawIntegerSerialiser"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

assumption that this is still ordered because that was the reason for removing the old RawIntegerSerialiser.

update the MultiSerialiser wanting to emphasise the effectiveness of it during the deprecation of serialisers.
8 is the last and best.
24 is not latest regardless of natural-order.
7 is dead.
@GCHQDev404 GCHQDev404 force-pushed the gh-2552-remove-deprecated-MultiSerialiser branch from 99b84e6 to fcb1d04 Compare February 14, 2022 16:44
@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2022

Codecov Report

Merging #2587 (6099934) into gh-2552-remove-deprecated (cf1d736) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@                       Coverage Diff                       @@
##             gh-2552-remove-deprecated    #2587      +/-   ##
===============================================================
- Coverage                        65.30%   65.28%   -0.02%     
+ Complexity                        2429     2427       -2     
===============================================================
  Files                              872      872              
  Lines                            27209    27209              
  Branches                          3165     3165              
===============================================================
- Hits                             17768    17764       -4     
- Misses                            8044     8046       +2     
- Partials                          1397     1399       +2     
Impacted Files Coverage Δ
.../serialisation/implementation/MultiSerialiser.java 52.38% <ø> (ø)
...onutil/iterable/LimitedInMemorySortedIterable.java 47.22% <0.00%> (-2.78%) ⬇️
.../java/uk/gov/gchq/gaffer/commonutil/OneOrMore.java 71.62% <0.00%> (-2.71%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf1d736...6099934. Read the comment docs.

Copy link
Contributor

@t92549 t92549 left a comment

Choose a reason for hiding this comment

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

Mostly just tedious spelling things, sorry!
Good content though.

Copy link
Member

@GCHQDeveloper314 GCHQDeveloper314 left a comment

Choose a reason for hiding this comment

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

These are good improvements. It's clear how the MultiSerialiser works and why it's useful. Only minor comments on formatting/spelling.

@t92549 t92549 merged commit 553412c into gh-2552-remove-deprecated Feb 18, 2022
@t92549 t92549 deleted the gh-2552-remove-deprecated-MultiSerialiser branch February 18, 2022 15:40
t92549 added a commit that referenced this pull request Feb 23, 2022
* gh-2552: Removed deprecated code except getTraits and hasTrait

* gh-2552: Fix javadoc issue

* gh-2571: Fixed matchedVertex behaviour and added new ITs (#2572)

* gh-2571: Fixed matchedVertex behaviour and added new ITs

* gh-2571: Fixed checkstyle

* gh-2571: Updated test name

* gh-2573: Replace hasTrait usages with Operation (#2574)

* gh-2573: Replace hasTrait usages with Operation

* gh-2573: Fix tests by making exception more explicit

* gh-2573: Fixed exception issues

* gh-2573: Simplified but still failing test

* gh-2573: Changed to user

* gh-2573: Changed FederatedGetTraitsHandler to return DYNAMIC_SCHEMA

* gh-2573: Use old function for FederatedOperationChainValidator

* gh-2573: Fixed FederatedStoreUtil

* gh-2573: Added Deprecation notice

* gh-2573: Cherry pick federated store tests fix from 2553

* Revert "gh-2573: Cherry pick federated store tests fix from 2553"

This reverts commit c367555.

* gh-2573: Manual fix of FederatedStoreGetTraitsTest cache

* gh-2573: Changed HasTraitHandler to not send Federated loop option

* Gh-2560 get traits operation configurable (#2579)

* Update docs links in README

* Revert "Update docs links in README"

This reverts commit 2d87bf3.

* gh-2559 added hasTrait, its handler and corresponding tests (#2561)

* gh-2559 added hasTrait, its handler and corresponding tests

* gh-2559 amended comment in HasTrait, added additional tests for the handler

* gh-2559 null check & tidy test

Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com>

* Update links to Gaffer docs in all files (#2566)

Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com>

* gh-2560 GetTraitsHandler is initalised with traits dependednt on store type

* gh-2560 reviewed comments

* gh-2560 unused imports

* gh-2560 review comments

* checkstyle

* gh-2560 fixed federated test

* gh-2560 fixed GraphTest

* gh-2560 fixed TestStore

* gh-2560-getTraitsOperation (#2582)

Removing redundant adding of GetTraitHandler from addAdditionalOperationHandlers() because it has its own method getGetTraitsHandler().

* gh-2560: Added fix to SingleUseMapStoreWithoutVisibilitySupport

* gh-2560: Fixed Set copy issue in GetTraitsHandler constructor

* gh-2560: Added extra protections to GetTraitsHandler

Co-authored-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com>
Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com>
Co-authored-by: GCHQDev404 <45399082+GCHQDev404@users.noreply.github.com>

* gh-2552: Removed some remaining parquet properties files

* gh-2552: Review comments traits test

* gh-2552-remove-deprecated MultiSerialiser (#2587)

* gh-2552-remove-deprecated MultiSerialiser

update the MultiSerialiser wanting to emphasise the effectiveness of it during the deprecation of serialisers.
8 is the last and best.
24 is not latest regardless of natural-order.
7 is dead.

* gh-2552: MultiSerialiser spelling corrections

* Update core/serialisation/src/main/java/uk/gov/gchq/gaffer/serialisation/implementation/MultiSerialiser.java

Co-authored-by: t92549 <80890692+t92549@users.noreply.github.com>

Co-authored-by: p3430233 <61738524+p3430233@users.noreply.github.com>
Co-authored-by: GCHQDeveloper314 <94527357+GCHQDeveloper314@users.noreply.github.com>
Co-authored-by: GCHQDev404 <45399082+GCHQDev404@users.noreply.github.com>
@t92549 t92549 linked an issue Feb 23, 2022 that may be closed by this pull request
@t92549 t92549 removed a link to an issue Feb 23, 2022
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