Skip to content

♻️ (frontend) do multiple refacto #949

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

Merged
merged 3 commits into from
May 12, 2025

Conversation

Zorin95670
Copy link
Contributor

Purpose

  • Simplify if statement
  • Simplify redundant conditional expression
  • Remove unnecessary local variable
  • Remove var usage instead of let/const
  • Add explicit React import to ensure JSX compatibility
  • Add test file to improve coverage
  • Add documentation

@Zorin95670 Zorin95670 force-pushed the improvement/code_warning branch from d94672a to 571c240 Compare May 7, 2025 14:04
@@ -1,4 +1,4 @@
var config = {
const config = {
Copy link

Choose a reason for hiding this comment

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

This can be enforced by the use of this eslint rule BTW:
https://eslint.org/docs/latest/rules/no-var

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add a commit that modify configuration to include root file.

@lunika lunika requested a review from AntoLC May 9, 2025 14:40
@Zorin95670 Zorin95670 force-pushed the improvement/code_warning branch 3 times, most recently from 1670302 to 5a82713 Compare May 12, 2025 09:13
Copy link
Collaborator

@AntoLC AntoLC left a comment

Choose a reason for hiding this comment

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

Hi, thank you for your contribution !

We try to keep meaningful commits in the history, could you squash them to have I think 3 commits ?

  • one about minor refactoring (condition etc..)
  • one about adding documentation
  • one about adding tests

Same about the changelog, only meaningful entries, maybe 1 about documentation and 1 about coverage improvement should be enough, wdyt ?

Thank you.

@@ -1,4 +1,4 @@
import { forwardRef } from 'react';
import React, { forwardRef } from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

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

We pin to "react19.1.0", so adding "React" is not necessary, same for the others.

"react": "19.1.0",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove commit that make this change!

@Zorin95670 Zorin95670 force-pushed the improvement/code_warning branch from 5a82713 to 33a1578 Compare May 12, 2025 09:31
- improve condition statements
- add "no-var" rule in eslint
- remove some unnecessary variables

Signed-off-by: Zorin95670 <[email protected]>
Improve and add jsdoc.

Signed-off-by: Zorin95670 <[email protected]>
Improve the test coverage of the "api" modules.

Signed-off-by: Zorin95670 <[email protected]>
@AntoLC AntoLC force-pushed the improvement/code_warning branch from 33a1578 to 29ea6b8 Compare May 12, 2025 12:08
@AntoLC AntoLC self-requested a review May 12, 2025 12:26
Copy link
Collaborator

@AntoLC AntoLC left a comment

Choose a reason for hiding this comment

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

Nice, thank you. 🎉

@AntoLC AntoLC merged commit 29ea6b8 into suitenumerique:main May 12, 2025
19 of 20 checks passed
@Zorin95670 Zorin95670 deleted the improvement/code_warning branch May 12, 2025 12:29
@AntoLC AntoLC mentioned this pull request May 22, 2025
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.

3 participants