Enhance Code Readability#84
Conversation
its better than filling with zero also use index range based on .net 8 new feature
change to correct namespace
…soft.com/en-us/dotnet/csharp/fundamentals/coding-style/identifier-names also i think camelCase is better naming convention in c# and has better readability
…e merge pattern in line 101
PaulusParssinen
left a comment
PaulusParssinen
left a comment
There was a problem hiding this comment.
Not a maintainer but otherwise I agree with all other changes 👍
|
I vote for this |
Co-authored-by: Paulus Pärssinen <[email protected]>
…erProvider.cs file
PaulusParssinen
left a comment
PaulusParssinen
left a comment
There was a problem hiding this comment.
Didn't suggest all rename changes so please go through the constants in LightEpoch again to make them consistent with my previous suggestion, otherwise LGTM 
Co-authored-by: Paulus Pärssinen <[email protected]>
Co-authored-by: Paulus Pärssinen <[email protected]>
and fix typo naming on method `BadInputNonExistingFile`
…Test and PasswordLessUserTest using range indexer
JeremyWu917
left a comment
JeremyWu917
left a comment
There was a problem hiding this comment.
I don't know if it makes sense, but it looks coooool 😎
Co-authored-by: Jeremy Wu <[email protected]>
Co-authored-by: Jeremy Wu <[email protected]>
Co-authored-by: Jeremy Wu <[email protected]>
|
What is the appetite for automatic styling? Many of these things can be linted for and even auto-fixed to some arbitrary level of consistency without requiring human intervention. |
|
I appreciate your point, but I don't think we can ignore the impact of human creativity on coding. |
TalZaccai
requested review from
TalZaccai
and removed request for
CallumTeesdale
|
@nimanikoo thank you so much for your contribution to Garnet, I can see that there was quite a bit of work that went into this and that people are overall happy with it. We would like to address most of these via our .editorconfig rules so that we're able to enforce them in the future and that we're all on the same page with regards to styling choices. We're planning on a code-style overhaul soon, and I will definitely use your PR for reference for desired choices. I'm therefore closing it for now... Thanks again! |
10 participants
Hello, I hope you are well
Thank you for helping to open source and build this project for the .NET community
Out of curiosity, I browsed through the code in this repository and found some hints that might be good.
One of the performance improvements was where you used lists.
I tried to use the new features of .NET 8 and C# 12 and refactored where needed
In the commits, I stated the reason for the changes and on what basis the changes were made.
In some files, it was necessary to sync namespace , and I think namespace scope can be used for all files, according to the rest of the new .NET projects such as Aspire.
The next thing is that you can use the primary constructor according to the new .NET 8 feature
In some cases, we can make the code more concise and readable and use expressions.
If this model of changes is approved by you, let me know so that I can apply it to the whole project and move forward step by step.
I hope these things are useful and I will be happy to contribute to the development of this project
Please check and let me know the result