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

Specify and enforce code-style using .editorconfig rules #142

Open
PaulusParssinen opened this issue Mar 26, 2024 · 1 comment
Open

Specify and enforce code-style using .editorconfig rules #142

PaulusParssinen opened this issue Mar 26, 2024 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@PaulusParssinen
Copy link
Contributor

PaulusParssinen commented Mar 26, 2024

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.

Couple examples of naming inconsistencies

readonly ClientSession<Key, Value, Input, Output, Context, Functions> clientSession;
readonly InternalTsavoriteSession TsavoriteSession;

readonly int BufferSize;
readonly SslClientAuthenticationOptions sslOptions;
Socket socket;
/// <summary>
/// Operation type
/// </summary>
public int opType;

internal int logRefCount = 1;
readonly ILogger logger;
/// <summary>
/// Whether we refresh safe tail as records are inserted
/// </summary>
readonly bool AutoRefreshSafeTailAddress;
/// <summary>
/// Callback when safe tail shifts
/// </summary>
public Action<long, long> SafeTailShiftCallback;
/// <summary>
/// Whether we automatically commit as records are inserted
/// </summary>
readonly bool AutoCommit;
/// <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)

@TalZaccai
Copy link
Contributor

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!

@TalZaccai TalZaccai self-assigned this Mar 26, 2024
@TalZaccai TalZaccai added the enhancement New feature or request label Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants