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

Figma: People Page Violates The Figma Style Guide #3171 #3274

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions .github/workflows/pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,16 @@ jobs:
id: changed-files
uses: tj-actions/changed-files@v45

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: 3.9
- name: Filter TypeScript files
id: filter-files
run: |
echo "ts_files=$(echo '${{ steps.changed-files.outputs.all_changed_files }}' | tr ',' '\n' | grep -E '\.(ts|tsx)$' | tr '\n' ' ')" >> $GITHUB_OUTPUT
shell: bash
Comment on lines +198 to +202
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix YAML syntax error and improve file filtering.

There's a syntax error in the YAML structure. Additionally, the file filtering could be more robust.

Apply this diff to fix the syntax and improve the filtering:

      - name: Filter TypeScript files
        id: filter-files
        run: |
-          echo "ts_files=$(echo '${{ steps.changed-files.outputs.all_changed_files }}' | tr ',' '\n' | grep -E '\.(ts|tsx)$' | tr '\n' ' ')" >> $GITHUB_OUTPUT
+          ts_files=$(echo '${{ steps.changed-files.outputs.all_changed_files }}' | tr ',' '\n' | grep -E '\.(ts|tsx)$' | tr '\n' ' ')
+          if [ -n "$ts_files" ]; then
+            echo "ts_files=$ts_files" >> $GITHUB_OUTPUT
+          fi
        shell: bash
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Filter TypeScript files
id: filter-files
run: |
echo "ts_files=$(echo '${{ steps.changed-files.outputs.all_changed_files }}' | tr ',' '\n' | grep -E '\.(ts|tsx)$' | tr '\n' ' ')" >> $GITHUB_OUTPUT
shell: bash
- name: Filter TypeScript files
id: filter-files
run: |
ts_files=$(echo '${{ steps.changed-files.outputs.all_changed_files }}' | tr ',' '\n' | grep -E '\.(ts|tsx)$' | tr '\n' ' ')
if [ -n "$ts_files" ]; then
echo "ts_files=$ts_files" >> $GITHUB_OUTPUT
fi
shell: bash
🧰 Tools
🪛 yamllint (1.35.1)

[error] 198-198: syntax error: expected , but found ''

(syntax)


- name: Run Python script
if: steps.filter-files.outputs.ts_files != ''
run: |
python .github/workflows/scripts/eslint_disable_check.py --files ${{ steps.changed-files.outputs.all_changed_files }}
python .github/workflows/scripts/eslint_disable_check.py --files ${{ steps.filter-files.outputs.ts_files }}
Comment on lines +205 to +207
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve conditional execution formatting.

The indentation of the if condition is incorrect, which could cause workflow failures.

-      if: steps.filter-files.outputs.ts_files != ''
+      if: ${{ steps.filter-files.outputs.ts_files != '' }}
        run: |
          python .github/workflows/scripts/eslint_disable_check.py --files ${{ steps.filter-files.outputs.ts_files }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if: steps.filter-files.outputs.ts_files != ''
run: |
python .github/workflows/scripts/eslint_disable_check.py --files ${{ steps.changed-files.outputs.all_changed_files }}
python .github/workflows/scripts/eslint_disable_check.py --files ${{ steps.filter-files.outputs.ts_files }}
if: ${{ steps.filter-files.outputs.ts_files != '' }}
run: |
python .github/workflows/scripts/eslint_disable_check.py --files ${{ steps.filter-files.outputs.ts_files }}


Check-Code-Coverage-Disable:
name: Check for code coverage disable
Expand Down
39 changes: 16 additions & 23 deletions .github/workflows/scripts/eslint_disable_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ def has_eslint_disable(file_path):
"""
# Initialize key variables
eslint_disable_pattern = re.compile(
r"\/\/\s*eslint-disable(?:-next-line"
r"|-line)?[^\n]*|\/\*\s*eslint-disable[^\*]*\*\/",
r"\/\/\s*eslint-disable(?:-next-line|-line)?[^\n]*|\/\*\s*eslint-disable[^\*]*\*\/",
re.IGNORECASE,
)

Expand All @@ -49,13 +48,14 @@ def has_eslint_disable(file_path):
return bool(eslint_disable_pattern.search(content))
except FileNotFoundError:
print(f"File not found: {file_path}")
return False

except PermissionError:
print(f"Permission denied: {file_path}")
return False

except (IOError, OSError) as e:
print(f"Error reading file {file_path}: {e}")
return False

return False


def check_eslint(files_or_directories):
Expand All @@ -71,30 +71,21 @@ def check_eslint(files_or_directories):

for item in files_or_directories:
if os.path.isfile(item):
# If it's a file, directly check it
if item.endswith(".ts") or item.endswith(".tsx"):
if has_eslint_disable(item):
print(
f"""\
File {item} contains eslint-disable statement. Please remove them and \
ensure the code adheres to the specified ESLint rules."""
)
eslint_found = True
# Check a single file
if item.endswith((".ts", ".tsx")) and has_eslint_disable(item):
print(f"Error: File {item} contains eslint-disable statements.")
eslint_found = True
elif os.path.isdir(item):
# If it's a directory, walk through it and check all
# .ts and .tsx files
# Recursively check files in a directory
for root, _, files in os.walk(item):
if "node_modules" in root:
continue
for file_name in files:
if file_name.endswith(".ts") or file_name.endswith(".tsx"):
if file_name.endswith((".ts", ".tsx")):
file_path = os.path.join(root, file_name)
if has_eslint_disable(file_path):
print(
f"""File {file_path} contains eslint-disable
statement."""
)
eslint_found = True
print(f"Error: File {file_path} contains eslint-disable statements.")
eslint_found = True

return eslint_found

Expand All @@ -107,7 +98,9 @@ def arg_parser_resolver():
Returns:
result: Parsed argument object
"""
parser = argparse.ArgumentParser()
parser = argparse.ArgumentParser(
description="Check TypeScript files for eslint-disable statements."
)
parser.add_argument(
"--files",
type=str,
Expand Down
1 change: 0 additions & 1 deletion .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

npm run format:fix
# npm run lint:fix
npm run lint-staged
Expand Down
4 changes: 4 additions & 0 deletions matrix.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
1 2 3 4 5
6 7 8 9 10
11 12 13 14 15
16 17 18 19 20
Loading
Loading