You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I'm opening this issue to get some input from the maintainers on the possibility of having the projects code-style enforced with .editorconfig rules.
Code-style such as naming conventions are very subjective and it would be nice if contributors could just do project wide dotnet format [--verify-no-changes] after doing their changes. This would reduce mental overhead of trying to deduce the correct naming for variables and fields (which are currently inconsistent, even across singular files in the project) and make future code reviews easier.
I'm most concerned about the inconsistency of the field and variable naming conventions and would like to suggest bumping all the dotnet_naming_rule.*.severity rules from suggestion to warning and then running dotnet format for the entire repository. This should avoid the manual work such as #84.
/// Whether we refresh safe tail as records are inserted
/// </summary>
readonlyboolAutoRefreshSafeTailAddress;
/// <summary>
/// Callback when safe tail shifts
/// </summary>
publicAction<long,long>SafeTailShiftCallback;
/// <summary>
/// Whether we automatically commit as records are inserted
/// </summary>
readonlyboolAutoCommit;
/// <summary>
/// Whether there is an ongoing auto refresh safe tail
/// </summary>
int_ongoingAutoRefreshSafeTailAddress=0;
Another thing that makes reading the code harder is the (inconsistenly) missing access and visibility modifiers from fields.
It's nice to see there is already effort to put into enforcing code-style already (see #106 etc.) but the naming would be nice to get under control early on 😄
I am myself big fan of the .NET Runtime code-style + file scoped namespaces (applying file scoped namespaces can be done with Visual Studio to the entire solution, but probably wise to time so that it would not cause big conflicts with inprogress PRs)
The text was updated successfully, but these errors were encountered:
Hey @PaulusParssinen! Thanks for your input :) I 100% agree, we unfortunately got to working on code-style errors very close to the OSS release date, so we tried to avoid wide and potentially invasive changes. This is definitely on the to-do list!
I'm opening this issue to get some input from the maintainers on the possibility of having the projects code-style enforced with .editorconfig rules.
Code-style such as naming conventions are very subjective and it would be nice if contributors could just do project wide
dotnet format [--verify-no-changes]
after doing their changes. This would reduce mental overhead of trying to deduce the correct naming for variables and fields (which are currently inconsistent, even across singular files in the project) and make future code reviews easier.I'm most concerned about the inconsistency of the field and variable naming conventions and would like to suggest bumping all the
dotnet_naming_rule.*.severity
rules fromsuggestion
towarning
and then runningdotnet format
for the entire repository. This should avoid the manual work such as #84.Couple examples of naming inconsistencies
garnet/libs/storage/Tsavorite/cs/src/core/ClientSession/LockableContext.cs
Lines 18 to 19 in 2aea8fb
garnet/libs/common/LightClient.cs
Lines 31 to 37 in 2aea8fb
garnet/libs/storage/Tsavorite/cs/src/core/TsavoriteLog/TsavoriteLog.cs
Lines 128 to 150 in 2aea8fb
Another thing that makes reading the code harder is the (inconsistenly) missing access and visibility modifiers from fields.
It's nice to see there is already effort to put into enforcing code-style already (see #106 etc.) but the naming would be nice to get under control early on 😄
I am myself big fan of the .NET Runtime code-style + file scoped namespaces (applying file scoped namespaces can be done with Visual Studio to the entire solution, but probably wise to time so that it would not cause big conflicts with inprogress PRs)
The text was updated successfully, but these errors were encountered: