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

* Refactored Document and TimingMetadata to not extend Attribute #1927

Open
wants to merge 22 commits into
base: integration
Choose a base branch
from

Conversation

ivakegg
Copy link
Collaborator

@ivakegg ivakegg commented Apr 11, 2023

  • Simplified any logic looking for nested documents

@ivakegg ivakegg force-pushed the feature/returnFieldExpressions branch 3 times, most recently from d7f8989 to 291895e Compare April 11, 2023 13:06
apmoriarty
apmoriarty previously approved these changes Apr 11, 2023
Copy link
Collaborator

@apmoriarty apmoriarty left a comment

Choose a reason for hiding this comment

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

LGTM. Nice cleanup.

@ivakegg ivakegg force-pushed the feature/returnFieldExpressions branch 2 times, most recently from d43de09 to fcfa3fa Compare April 11, 2023 13:30
apmoriarty
apmoriarty previously approved these changes Apr 11, 2023
apmoriarty
apmoriarty previously approved these changes Apr 12, 2023
Copy link
Collaborator

@jwomeara jwomeara left a comment

Choose a reason for hiding this comment

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

I kind of rushed through this at the end once i got to the tests, but I had a few comments.

apmoriarty
apmoriarty previously approved these changes May 2, 2023
jwomeara
jwomeara previously approved these changes May 2, 2023
@ivakegg ivakegg dismissed stale reviews from apmoriarty and jwomeara via a781aac September 22, 2023 12:00
@ivakegg ivakegg force-pushed the feature/returnFieldExpressions branch from 8cdc7b8 to a781aac Compare September 22, 2023 12:00
@ivakegg ivakegg force-pushed the feature/returnFieldExpressions branch from e83d48d to f86e38d Compare October 4, 2023 12:51
* Simplified any logic looking for nested documents
* Ensure everything is actually serializable
* Made the projection testing a little more robust
* Updated to avoid creating a copy of the document
@ivakegg ivakegg force-pushed the feature/returnFieldExpressions branch from baf4e1b to e46c7a1 Compare November 22, 2023 18:44
@billoley billoley self-requested a review December 20, 2023 16:34
@billoley
Copy link
Collaborator

billoley commented Dec 20, 2023

In DocumentProjection, the following constructor arguments and class memebers are no longer used. Is this intentional?
If so, they should probably be removed from the constructor and the class.

private final boolean includeGroupingContext;
private final boolean reducedResponse;    
private boolean trackSizes = true;

public DocumentProjection(boolean includeGroupingContext, boolean reducedResponse, boolean trackSizes, Set<String> projections, Projection.ProjectionType projectionType) {
    this.includeGroupingContext = includeGroupingContext;         // no longer used
    this.reducedResponse = reducedResponse;                            // no longer used
    this.projection = new Projection(projections, projectionType);
    this.trackSizes = trackSizes;                                                      // no longer used
}

@ivakegg
Copy link
Collaborator Author

ivakegg commented Feb 14, 2024

@billoley Cleaned up those members and constructors. Thanks!

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