ChenKaiLiuG/life_generators
1
0
Fork 0
Code Issues 7 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Chore(Fortune): add pre-commit to check special events #55

Merged
tobiichi3227 merged 3 commits from ci/event-check-pre-commit into main 2025-02-20 17:00:49 +00:00
Conversation 17 Commits 3 Files Changed 1 +36
tobiichi3227 commented 2025-02-17 04:12:57 +00:00 (Migrated from github.com)
Copy Link
No description provided.
lifeadventurer (Migrated from github.com) requested changes 2025-02-17 15:20:24 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
lifeadventurer (Migrated from github.com) commented 2025-02-17 12:52:12 +00:00
Copy Link

Use the latest version v5.0.0

Use the latest version v5.0.0
lifeadventurer (Migrated from github.com) commented 2025-02-17 15:20:05 +00:00
Copy Link

I noticed you're using a shell script wrapper to run the Python scripts. Since check-events.py is a Python program, I'd recommend calling it directly rather than through a shell script wrapper.
This approach would be more Windows-friendly, as shell scripts can be problematic on Windows environments. Making this change will ensure developers across all platforms can use the pre-commit hooks without issue.

I noticed you're using a shell script wrapper to run the Python scripts. Since `check-events.py` is a Python program, I'd recommend calling it directly rather than through a shell script wrapper. This approach would be more Windows-friendly, as shell scripts can be problematic on Windows environments. Making this change will ensure developers across all platforms can use the pre-commit hooks without issue.
tobiichi3227 (Migrated from github.com) reviewed 2025-02-18 02:45:36 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
tobiichi3227 (Migrated from github.com) commented 2025-02-18 02:45:36 +00:00
Copy Link

I switched to using a Python script because passing parameters directly is difficult.

I switched to using a Python script because passing parameters directly is difficult.
lifeadventurer (Migrated from github.com) reviewed 2025-02-18 06:36:11 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
lifeadventurer (Migrated from github.com) commented 2025-02-18 06:36:11 +00:00
Copy Link

Isn't this working?
entry: python3 scripts/check-events.py fortune_generator/json/custom_special.json custom

Isn't this working? `entry: python3 scripts/check-events.py fortune_generator/json/custom_special.json custom`
tobiichi3227 (Migrated from github.com) reviewed 2025-02-18 06:49:14 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
tobiichi3227 (Migrated from github.com) commented 2025-02-18 06:49:14 +00:00
Copy Link

Oh.
It works.
But I think this way can check three files and list all errors.
Instead of checking after each modification.

Oh. It works. But I think this way can check three files and list all errors. Instead of checking after each modification.
lifeadventurer (Migrated from github.com) reviewed 2025-02-18 06:53:37 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
lifeadventurer (Migrated from github.com) commented 2025-02-18 06:53:37 +00:00
Copy Link

What do you mean by "checking after each modification"?

What do you mean by "checking after each modification"?
tobiichi3227 (Migrated from github.com) reviewed 2025-02-18 06:57:03 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
tobiichi3227 (Migrated from github.com) commented 2025-02-18 06:57:02 +00:00
Copy Link

Modify, commit, and then pre-commit check.

Modify, commit, and then pre-commit check.
lifeadventurer (Migrated from github.com) reviewed 2025-02-18 07:01:12 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
lifeadventurer (Migrated from github.com) commented 2025-02-18 07:01:12 +00:00
Copy Link

Why do you need to check after each modification?

Why do you need to check after each modification?
lifeadventurer (Migrated from github.com) reviewed 2025-02-18 07:04:59 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
lifeadventurer (Migrated from github.com) commented 2025-02-18 07:04:59 +00:00
Copy Link
entry: |
      python3 scripts/check-events.py fortune_generator/json/custom_special.json custom &&
      python3 scripts/check-events.py fortune_generator/json/cyclical_special.json cyclical &&
      python3 scripts/check-events.py fortune_generator/json/static_special.json static
``` entry: | python3 scripts/check-events.py fortune_generator/json/custom_special.json custom && python3 scripts/check-events.py fortune_generator/json/cyclical_special.json cyclical && python3 scripts/check-events.py fortune_generator/json/static_special.json static ```
tobiichi3227 (Migrated from github.com) reviewed 2025-02-18 07:13:15 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
tobiichi3227 (Migrated from github.com) commented 2025-02-18 07:13:15 +00:00
Copy Link

It doesn't work.

It doesn't work.
tobiichi3227 (Migrated from github.com) reviewed 2025-02-18 07:14:08 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
tobiichi3227 (Migrated from github.com) commented 2025-02-18 07:14:07 +00:00
Copy Link
bash -c '
  python3 scripts/check-events.py fortune_generator/json/custom_special.json custom &&
  python3 scripts/check-events.py fortune_generator/json/cyclical_special.json cyclical &&
  python3 scripts/check-events.py fortune_generator/json/static_special.json static
'

This can work.
However, it's not possible to check all three files at the same time.

```bash bash -c ' python3 scripts/check-events.py fortune_generator/json/custom_special.json custom && python3 scripts/check-events.py fortune_generator/json/cyclical_special.json cyclical && python3 scripts/check-events.py fortune_generator/json/static_special.json static ' ``` This can work. However, it's not possible to check all three files at the same time.
lifeadventurer (Migrated from github.com) reviewed 2025-02-18 07:16:37 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
lifeadventurer (Migrated from github.com) commented 2025-02-18 07:16:37 +00:00
Copy Link

Set language: system instead of language: python because we're utilizing shell functionality rather than running a standalone Python script.

Set `language: system` instead of `language: python` because we're utilizing shell functionality rather than running a standalone Python script.
lifeadventurer (Migrated from github.com) reviewed 2025-02-18 07:17:10 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
lifeadventurer (Migrated from github.com) commented 2025-02-18 07:17:10 +00:00
Copy Link
bash -c '
  python3 scripts/check-events.py fortune_generator/json/custom_special.json custom &&
  python3 scripts/check-events.py fortune_generator/json/cyclical_special.json cyclical &&
  python3 scripts/check-events.py fortune_generator/json/static_special.json static
'

This can work. However, it's not possible to check all three files at the same time.

We have concluded that we should not use bash.

> ```shell > bash -c ' > python3 scripts/check-events.py fortune_generator/json/custom_special.json custom && > python3 scripts/check-events.py fortune_generator/json/cyclical_special.json cyclical && > python3 scripts/check-events.py fortune_generator/json/static_special.json static > ' > ``` > > This can work. However, it's not possible to check all three files at the same time. We have concluded that we should not use bash.
tobiichi3227 (Migrated from github.com) reviewed 2025-02-18 07:18:41 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
tobiichi3227 (Migrated from github.com) commented 2025-02-18 07:18:41 +00:00
Copy Link

Set language: system instead of language: python because we're utilizing shell functionality rather than running a standalone Python script.

Yes.
I already setted

> Set `language: system` instead of `language: python` because we're utilizing shell functionality rather than running a standalone Python script. Yes. I already setted
tobiichi3227 (Migrated from github.com) reviewed 2025-02-18 07:19:19 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
tobiichi3227 (Migrated from github.com) commented 2025-02-18 07:19:19 +00:00
Copy Link
bash -c '
  python3 scripts/check-events.py fortune_generator/json/custom_special.json custom &&
  python3 scripts/check-events.py fortune_generator/json/cyclical_special.json cyclical &&
  python3 scripts/check-events.py fortune_generator/json/static_special.json static
'

This can work. However, it's not possible to check all three files at the same time.

We have concluded that we should not use bash.

Yes

> > ```shell > > bash -c ' > > python3 scripts/check-events.py fortune_generator/json/custom_special.json custom && > > python3 scripts/check-events.py fortune_generator/json/cyclical_special.json cyclical && > > python3 scripts/check-events.py fortune_generator/json/static_special.json static > > ' > > ``` > > > > > > > > > > > > > > > > > > > > > > > > This can work. However, it's not possible to check all three files at the same time. > > We have concluded that we should not use bash. Yes
lifeadventurer (Migrated from github.com) reviewed 2025-02-18 07:23:08 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
lifeadventurer (Migrated from github.com) commented 2025-02-18 07:23:08 +00:00
Copy Link

Set language: system instead of language: python because we're utilizing shell functionality rather than running a standalone Python script.

Yes. I already setted

What is the problem of it not working?

> > Set `language: system` instead of `language: python` because we're utilizing shell functionality rather than running a standalone Python script. > > Yes. I already setted What is the problem of it not working?
tobiichi3227 (Migrated from github.com) reviewed 2025-02-18 07:56:21 +00:00
.pre-commit-config.yaml
@@ -0,0 +1,36 @@
# See https://pre-commit.com for more information
tobiichi3227 (Migrated from github.com) commented 2025-02-18 07:56:21 +00:00
Copy Link

Changing to language: system allows it to work properly.

Changing to language: system allows it to work properly.
lifeadventurer (Migrated from github.com) approved these changes 2025-02-20 17:00:36 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
tobiichi3227
lifeadventurer
Labels
Clear labels
bug
Something isn't working
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 question
Further information is requested
 stale
Pull Request is stale
 waiting for merge
waiting for review
waiting for update
wontfix
This will not be worked on
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
ChenKaiLiuG
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: ChenKaiLiuG/life_generators#55
Delete Branch "ci/event-check-pre-commit"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?