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

changes made for Redundant jump statements #684 #724

Merged
merged 4 commits into from
Oct 28, 2022

Conversation

sachdevlaksh
Copy link
Contributor

Changes provided for Issue #684

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.

While you did follow the instructions that were provided in #684, they were unfortunately not detailed enough (our fault). Please make the following changes to every changed line:

  1. The "link to related SonarCloud issue" was meant to be a permanent link to a SonarCloud document that describes the issue (similar to how Microsoft does CA1812). Unfortunately, no such document for "SonarCloud S3626" comes up in a Google search, I had to search for "Code Analysis S3626" to get a hit: https://rules.sonarsource.com/csharp/RSPEC-3626. This SonarCloud scan setup is temporary, not a permanent URL that we should put in the source code. We are working on an official SonarCloud setup in ci: Added sonar workflow #709, but it isn't ready yet.
  2. Do not add an additional line for the comment, place the comment after the code that is commented on the same line. Include the // also.

Example

// continue; // LUCENENET: Removed redundant jump statements. https://rules.sonarsource.com/csharp/RSPEC-3626

src/Lucene.Net.Suggest/Suggest/DocumentDictionary.cs Outdated Show resolved Hide resolved
src/Lucene.Net/Codecs/BlockTreeTermsReader.cs Outdated Show resolved Hide resolved
src/Lucene.Net/Codecs/Compressing/LZ4.cs Outdated Show resolved Hide resolved
src/Lucene.Net/Codecs/MultiLevelSkipListReader.cs Outdated Show resolved Hide resolved
src/Lucene.Net/Support/IO/FileSupport.cs Outdated Show resolved Hide resolved
src/Lucene.Net/Util/Fst/FSTEnum.cs Outdated Show resolved Hide resolved
src/Lucene.Net/Util/Fst/FSTEnum.cs Outdated Show resolved Hide resolved
@sachdevlaksh
Copy link
Contributor Author

Thanks for the PR.

While you did follow the instructions that were provided in #684, they were unfortunately not detailed enough (our fault). Please make the following changes to every changed line:

  1. The "link to related SonarCloud issue" was meant to be a permanent link to a SonarCloud document that describes the issue (similar to how Microsoft does CA1812). Unfortunately, no such document for "SonarCloud S3626" comes up in a Google search, I had to search for "Code Analysis S3626" to get a hit: https://rules.sonarsource.com/csharp/RSPEC-3626. This SonarCloud scan setup is temporary, not a permanent URL that we should put in the source code. We are working on an official SonarCloud setup in ci: Added sonar workflow #709, but it isn't ready yet.
  2. Do not add an additional line for the comment, place the comment after the code that is commented on the same line. Include the // also.

Example

// continue; // LUCENENET: Removed redundant jump statements. https://rules.sonarsource.com/csharp/RSPEC-3626

Thanks @NightOwl888, For the detail, I make changes asap.

@NightOwl888 NightOwl888 merged commit 22cac25 into apache:master Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants