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

Documentation improvements: Encryption #494

Merged

Conversation

jescalada
Copy link
Contributor

This PR adds summaries to public methods and fields in Encryption-related files. Eventually, we want to generate an HTML API reference, so the documentation should be as thorough as possible.

I went through TestEncryption.cs, checked all the public-facing classes, and documented the methods. For some methods, I used pyarrow documentation as a reference, for others I just used common sense (and Copilot to match existing doc styles). I'm not sure what degree of C# literacy we are expecting (such as explaining or omitting that certain classes must be used with using keyword).

Please let me know about any inaccuracies/redundancies or how to improve the docs! I'll use the feedback to document some of the larger files more accurately later on (such as the core Reader/Writer classes). Thank you!

Copy link
Contributor

@adamreeve adamreeve left a comment

Choose a reason for hiding this comment

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

This looks great thanks @jescalada, I've left some mostly minor comments

csharp/ColumnDecryptionPropertiesBuilder.cs Outdated Show resolved Hide resolved
csharp/ColumnEncryptionPropertiesBuilder.cs Outdated Show resolved Hide resolved
csharp/ColumnEncryptionPropertiesBuilder.cs Outdated Show resolved Hide resolved
csharp/FileDecryptionPropertiesBuilder.cs Outdated Show resolved Hide resolved
csharp/FileDecryptionPropertiesBuilder.cs Outdated Show resolved Hide resolved
csharp/FileEncryptionProperties.cs Outdated Show resolved Hide resolved
csharp/RowGroupMetaData.cs Outdated Show resolved Hide resolved
csharp/RowGroupMetaData.cs Outdated Show resolved Hide resolved
@jescalada jescalada requested a review from adamreeve January 24, 2025 04:30
Copy link
Contributor

@adamreeve adamreeve left a comment

Choose a reason for hiding this comment

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

🚀

@adamreeve adamreeve merged commit 36e8ccd into G-Research:master Jan 24, 2025
31 checks passed
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.

2 participants