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

Improve Code Quality #754

Closed
wants to merge 3 commits into from
Closed

Improve Code Quality #754

wants to merge 3 commits into from

Conversation

iamcarbon
Copy link
Contributor

  • Enables C#10
  • Use null propagation
  • Removes various unused using statements

@iamcarbon
Copy link
Contributor Author

Ready for review.

Copy link
Contributor

@NightOwl888 NightOwl888 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

There is just one requested change to make, please see the inline comments.

Also, please be aware that this is a port from Java, and generally these "code quality" changes are rejected because although they may make the code more easy to read for a C# developer, they make us do more mental gymnastics when comparing against the original source. We are planning to upgrade to a newer version of Lucene and the more lines that differ, the harder that will be to do. We will accept this change because it generally doesn't disrupt the flow and is easy to comprehend.

Just so we don't have to reject any future PRs because they diverge too far from the original source, please either work on something that is up for grabs, or ask whether it is a change that will be accepted on the dev mailing list prior to doing the work.

@@ -26,7 +26,7 @@
</PropertyGroup>

<PropertyGroup>
<LangVersion>9.0</LangVersion>
<LangVersion>10</LangVersion>
Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks support for VS2019, which we are still using. Please revert.

At least until someone upgrades Git Tools 2019 to support VS2022. See yysun/git-tools#40.

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