Skip to content

Fix issues with sonarcolud #1370#1395

Open
sinproject-iwasaki wants to merge 4 commits intomainfrom
1370-fix-issues-with-sonarcolud
Open

Fix issues with sonarcolud #1370#1395
sinproject-iwasaki wants to merge 4 commits intomainfrom
1370-fix-issues-with-sonarcolud

Conversation

@sinproject-iwasaki
Copy link
Contributor

@sinproject-iwasaki sinproject-iwasaki commented Sep 15, 2023

closes #1370

For the Submitter

Tasks

  • Make sure "closes (issue number)" is written close to the top
  • Attach the enhancement label for update tasks
  • Make sure CI completes successfully
  • Check everything written in the PR
  • Fix the issues pointed out by AI
  • Add reviewers
  • On Slack, mention the PR reviewer, write your thoughts on the issue, and inform them that the PR has been created

For the Reviewer

Before Reviewing

  • Write which PR you are reviewing on Slack

Checks

  • Each task for the related issue are written and in checkboxes
  • The branch name is close to the issue name
  • CI completes successfully
  • Changes include only what is necessary for this PR
  • Necessary unit tests are implemented
  • Necessary E2E tests are implemented
  • Perform functionality checks
  • If there are any problems, request revisions from the PR submitter and mention them on Slack

After Reviewing

  • Leave an approval comment (e.g. LGTM)
  • On Slack, mention the PR submitter and a project admin, write which PR you have reviewed, and ask them to merge the PR

@qlty-cloud-legacy
Copy link

Code Climate has analyzed commit 570d685 and detected 4 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 4

View more on Code Climate.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link

Update Summary

The diff points to testing and improvement of auth setup and sign in feature, updating sign in button, improving conditional logic to "change locale" and adding some more tests that were previously commented out.

auth.setup.ts

  • Unnecessary code that are commented out has been removed.

learn.spec.ts

  • Uncommented and updated test scenario for changing locale.

translate.spec.ts

  • Test for moving text into textbox from history has been uncommented and implemented.
  • Test for displaying the translation after text is added is only performed if not in CI environment.

get_pin_code_from_mail.ts

  • Code readability has been improved using optional chaining.

logger.ts

  • Some commented out logging functionality has been removed.

pin_code.ts

  • Validation for pin code length being less than 6 has been uncommented.

text_repository.ts and text_repository_prisma.ts

  • Removed unnecessary function 'find_unique' that was defined but not being used.

Other files

  • More changes were mainly related to removing unused TODOs and commented code.

Suggestions for improvement

auth.setup.ts

  • Implement scenario-based comments such as functionality by each function and purpose.

learn.spec.ts

  • Implement validation tests for On failure scenarios, such as when network error occurs during locale change.

translate.spec.ts

  • A unit test regarding translation functionality rather than a test case which is dependent on external environment should be implemented.

Other files

  • Remove unused and commented code to make your code cleaner.
  • Using optional chaining and nullish coalescing operator can make your code more readable.
  • If there is part of the code that is commented out such as a feature to be implemented in the future, it would be better to create an issue in the issue tracker and manage it rather than in the code.

@REI-MORI REI-MORI requested review from Acha0203 and milkyskies and removed request for Acha0203 October 21, 2023 02:28
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.

Fix issues with SonarCloud

1 participant