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.

Sign up for GitHub

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

chore: Add Taskfile tasks to lint YAML files. #4

Merged
merged 2 commits into from
Oct 30, 2024
Merged

chore: Add Taskfile tasks to lint YAML files. #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
056256f
chore: Add Taskfile tasks to lint YAML files.
kirkrodrigues Oct 30, 2024
8a2077f
Update README.md
kirkrodrigues Oct 30, 2024
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
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Build-related directories and files
.task/
target/
3 changes: 3 additions & 0 deletions .gitmodules
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[submodule "tools/yscope-dev-utils"]
path = tools/yscope-dev-utils
url = https://github.com/y-scope/yscope-dev-utils.git
39 changes: 39 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# log4j2-appenders
This is a repository for a set of useful [Log4j 2][log4j2] appenders.

# Contributing
Follow the steps below to develop and contribute to the project.

## Set up
Initialize and update the submodules:
```shell
git submodule update --init --recursive
```

## Linting
Before submitting a pull request, ensure you’ve run the linting [commands](#running-the-linters)
below and either fixed any violations or suppressed the warnings.

### Requirements
* Python 3.10 or higher
* [Task] 3.38.0 or higher

### Adding files
Certain file types need to be added to our linting rules manually:

* If adding a **YAML** file (regardless of its extension), add it as an argument to the `yamllint`
command in [lint-tasks.yaml](lint-tasks.yaml).

### Running the linters
To run all linting checks:
```shell
task lint:check
```

To run all linting checks AND automatically fix any fixable issues:
```shell
task lint:fix
```

[log4j2]: https://logging.apache.org/log4j/2.x/index.html
[Task]: https://taskfile.dev
1 change: 1 addition & 0 deletions lint-requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
yamllint>=1.35.1
58 changes: 58 additions & 0 deletions lint-tasks.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
version: "3"

vars:
G_LINT_VENV_DIR: "{{.G_BUILD_DIR}}/lint-venv"
G_LINT_VENV_CHECKSUM_FILE: "{{.G_BUILD_DIR}}/lint#venv.md5"

tasks:
check:
cmds:
- task: "yml-check"

fix:
cmds:
- task: "yml-fix"

yml:
aliases:
- "yml-check"
- "yml-fix"
deps: ["venv"]
cmds:
- |-
. "{{.G_LINT_VENV_DIR}}/bin/activate"
yamllint \
--config-file "tools/yscope-dev-utils/lint-configs/.yamllint.yml" \
--strict \
.github/ \
lint-tasks.yaml \
taskfile.yaml

Comment on lines +16 to +30
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider making YAML paths configurable

The list of YAML files to lint is hardcoded. Consider moving these paths to variables for better maintainability.

Here's a suggested improvement:

-          .github/ \
-          lint-tasks.yaml \
-          taskfile.yaml
+          {{.YAML_PATHS}}

Add to the vars section:

vars:
  YAML_PATHS: .github/ lint-tasks.yaml taskfile.yaml

venv:
internal: true
vars:
CHECKSUM_FILE: "{{.G_LINT_VENV_CHECKSUM_FILE}}"
OUTPUT_DIR: "{{.G_LINT_VENV_DIR}}"
sources:
- "{{.ROOT_DIR}}/taskfile.yaml"
- "{{.TASKFILE}}"
- "lint-requirements.txt"
generates: ["{{.CHECKSUM_FILE}}"]
run: "once"
deps:
- ":init"
- task: ":utils:validate-checksum"
vars:
CHECKSUM_FILE: "{{.CHECKSUM_FILE}}"
DATA_DIR: "{{.OUTPUT_DIR}}"
cmds:
- task: ":utils:create-venv"
vars:
LABEL: "lint"
OUTPUT_DIR: "{{.OUTPUT_DIR}}"
REQUIREMENTS_FILE: "lint-requirements.txt"
# This command must be last
- task: ":utils:compute-checksum"
vars:
DATA_DIR: "{{.OUTPUT_DIR}}"
OUTPUT_FILE: "{{.CHECKSUM_FILE}}"
19 changes: 19 additions & 0 deletions taskfile.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
version: "3"

includes:
lint: "lint-tasks.yaml"
utils: "tools/yscope-dev-utils/taskfiles/utils.yml"

Comment on lines +1 to +6
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Fix missing utility task file and standardize YAML file extensions

The verification revealed:

  • lint-tasks.yaml exists and is correctly referenced
  • tools/yscope-dev-utils/taskfiles/utils.yml is missing
  • Inconsistent YAML extensions in the project (both .yaml and .yml are used)

Please:

  • Ensure the yscope-dev-utils submodule is properly initialized and updated
  • Consider standardizing to .yaml extension for consistency with existing files
🔗 Analysis chain

Verify the included task file paths and standardize file extensions.

The file paths need verification, and there's an inconsistency in the YAML file extensions (.yaml vs .yml).

Let's verify the file paths and check for consistency:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence of included task files and check for YAML extension consistency

echo "Checking for included task files..."
for file in lint-tasks.yaml tools/yscope-dev-utils/taskfiles/utils.yml; do
  if [ -f "$file" ]; then
    echo "✓ Found: $file"
  else
    echo "✗ Missing: $file"
  fi
done

echo -e "\nChecking YAML file extension consistency..."
fd -e yml -e yaml . -x echo "Found YAML file: {}"

Length of output: 710

vars:
G_BUILD_DIR: "{{.ROOT_DIR}}/target"

tasks:
clean:
cmds:
- "rm -rf '{{.G_BUILD_DIR}}'"

Comment on lines +11 to +14
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding a precondition check to the clean task.

While the clean task is well-implemented, it could benefit from a precondition to verify the directory exists before attempting removal.

Consider this safer implementation:

  clean:
    cmds:
-     - "rm -rf '{{.G_BUILD_DIR}}'"
+     - |
+       if [ -d "{{.G_BUILD_DIR}}" ]; then
+         rm -rf "{{.G_BUILD_DIR}}"
+       else
+         echo "Directory {{.G_BUILD_DIR}} does not exist. Nothing to clean."
+       fi
📝 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
clean:
cmds:
- "rm -rf '{{.G_BUILD_DIR}}'"
clean:
cmds:
- |
if [ -d "{{.G_BUILD_DIR}}" ]; then
rm -rf "{{.G_BUILD_DIR}}"
else
echo "Directory {{.G_BUILD_DIR}} does not exist. Nothing to clean."
fi

init:
internal: true
silent: true
run: "once"
cmds: ["mkdir -p '{{.G_BUILD_DIR}}'"]
1 change: 1 addition & 0 deletions tools/yscope-dev-utils
Open in desktop
Submodule yscope-dev-utils added at 159768