Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make FieldName Properties Public #353

Merged
merged 1 commit into from
Dec 25, 2024

Conversation

zhskay
Copy link
Contributor

@zhskay zhskay commented Dec 19, 2024

Title

Make FieldName Properties Public

Description

This PR updates the FieldName class by making its properties public. This allows easier comparison of attributes between existing and new Redis indexes before creating, simplifying validation workflows.

Changes

  • Made all FieldName properties public.

Impact

The change is backward-compatible and improves developer efficiency in index comparisons.

@atakavci
Copy link
Collaborator

atakavci commented Dec 24, 2024

hi @zhskay , thank you for your interest in NRedisStack.
could you explain your use case a bit more as well as the reasoning for protected set with alias property ?

@zhskay
Copy link
Contributor Author

zhskay commented Dec 25, 2024

Hello @atakavci, thank you for your feedback.
I have some logic that looks like this:

Schema schema = new();  
foreach (var item in items)  
{  
    // Add fields to the schema with type, name, and other properties based on the item content  
}  

After constructing the schema, I need to compare the existing index with the schema's attributes:

foreach (var field in schema.Fields)  
{  
    var attribute = existingIndex.Attributes.FirstOrDefault(a => // Find attribute by field name);  
    // Compare the attribute with the field  
}  

The issue is that I currently have no way to access the field name for comparison.

Regarding the Alias property, it can have a private set. Thanks for pointing that out.

@zhskay zhskay force-pushed the MadeFieldNamePropertiesPublic branch from 5a56187 to 83a0740 Compare December 25, 2024 07:25
Copy link
Collaborator

@atakavci atakavci left a comment

Choose a reason for hiding this comment

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

ideally each PR code requires the addition/modification of relevant tests, better with integration and unit tests. Since this is pretty straightforward with just exposing existing fields i believe there is not much to test.

thank your contribution and for considering to change alias as private.
LGTM.

@atakavci atakavci merged commit 8246616 into redis:master Dec 25, 2024
18 checks passed
@atakavci atakavci added the enhancement New feature or request label Jan 23, 2025
atakavci pushed a commit that referenced this pull request Jan 31, 2025
Made FieldName properties public
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants