JOJ/JOJ3-config-generator
27
1
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

dev #10

Merged
李衍志523370910113 merged 238 commits from dev into master 2025-03-05 16:20:39 +08:00
Conversation 16 Commits 238 Files Changed 42 +28 -28
3 changed files with 28 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 635405ea2b - Show all commits

6
joj3_config_generator/convert.py
View File

@ -1,5 +1,5 @@
import os
from typing import List
from typing import Set
from joj3_config_generator.models import joj1, repo, result, task
from joj3_config_generator.models.const import CACHE_ROOT, JOJ3_CONFIG_ROOT
@ -35,10 +35,10 @@ def convert(repo_conf: repo.Config, task_conf: task.Config) -> result.Config:
# Construct health check stage
if not repo_conf.force_skip_health_check_on_test or not current_test:
result_conf.stage.stages.append(get_health_check_config(repo_conf))
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-03 03:43:53 +08:00
Outdated
Review
Copy Link

where is it used?

where is it used?
李衍志523370910113 commented 2025-03-03 10:31:09 +08:00
Outdated
Review
Copy Link

this should be storing all the files that are about to be copy in or out

this should be storing all the files that are about to be copy in or out
李衍志523370910113 commented 2025-03-03 10:33:32 +08:00
Outdated
Review
Copy Link

It is as the input and output for the following functions about parsers

It is as the input and output for the following functions about parsers
张泊明518370910136 commented 2025-03-03 10:33:33 +08:00
Outdated
Review
Copy Link

so this feature is not implemented?

so this feature is not implemented?
李衍志523370910113 commented 2025-03-03 10:38:31 +08:00
Outdated
Review
Copy Link
    if not repo_conf.force_skip_health_check_on_test or not current_test:
        result_conf.stage.stages.append(get_health_check_config(repo_conf))
    cached: List[str] = []
    # Convert each stage in the task configuration
    for task_stage in task_conf.stages:
        executor_with_config, cached = get_executor_with_config(task_stage, cached)
        conf_stage = get_conf_stage(task_stage, executor_with_config)
        conf_stage = fix_result_detail(task_stage, conf_stage)
        conf_stage = fix_dummy(task_stage, conf_stage)
        conf_stage = fix_keyword(task_stage, conf_stage)
        conf_stage = fix_file(task_stage, conf_stage)
        conf_stage = fix_diff(task_stage, conf_stage, task_conf)
        result_conf.stage.stages.append(conf_stage)

it is

    for task_stage in task_conf.stages:
        executor_with_config, cached = get_executor_with_config(task_stage, cached)

this is a loop, so this cached will be updated in every round of stage

```python if not repo_conf.force_skip_health_check_on_test or not current_test: result_conf.stage.stages.append(get_health_check_config(repo_conf)) cached: List[str] = [] # Convert each stage in the task configuration for task_stage in task_conf.stages: executor_with_config, cached = get_executor_with_config(task_stage, cached) conf_stage = get_conf_stage(task_stage, executor_with_config) conf_stage = fix_result_detail(task_stage, conf_stage) conf_stage = fix_dummy(task_stage, conf_stage) conf_stage = fix_keyword(task_stage, conf_stage) conf_stage = fix_file(task_stage, conf_stage) conf_stage = fix_diff(task_stage, conf_stage, task_conf) result_conf.stage.stages.append(conf_stage) ``` it is ```python for task_stage in task_conf.stages: executor_with_config, cached = get_executor_with_config(task_stage, cached) ``` this is a loop, so this `cached` will be updated in every round of stage
张泊明518370910136 commented 2025-03-03 10:44:19 +08:00
Outdated
Review
Copy Link

The return value is unnecessary.

The return value is unnecessary.
李衍志523370910113 commented 2025-03-03 10:48:07 +08:00
Outdated
Review
Copy Link

I have a lazing coding style here, everything has get imported would get exported, so should maintain this until the end of the loop. Everything is exported in previous stage would be imported in the next stage.

I have a lazing coding style here, everything has get imported would get exported, so should maintain this until the end of the loop. Everything is exported in previous stage would be imported in the next stage.
张泊明518370910136 commented 2025-03-03 10:55:00 +08:00
Outdated
Review
Copy Link
  1. The return value is unnecessary
  2. It should be a set
1. The return value is unnecessary 2. It should be a `set`
张泊明518370910136 commented 2025-03-03 10:58:32 +08:00
Outdated
Review
Copy Link

try it yourself

try it yourself
李衍志523370910113 commented 2025-03-03 11:05:15 +08:00
Outdated
Review
Copy Link

I see why

I see why
李衍志523370910113 commented 2025-03-03 11:09:22 +08:00
Outdated
Review
Copy Link

resolved.

resolved.
cached: List[str] = []
cached: Set[str] = set()
# Convert each stage in the task configuration
for task_stage in task_conf.stages:
executor_with_config, cached = get_executor_with_config(task_stage, cached)
executor_with_config = get_executor_with_config(task_stage, cached)
conf_stage = get_conf_stage(task_stage, executor_with_config)
conf_stage = fix_result_detail(task_stage, conf_stage)
conf_stage = fix_dummy(task_stage, conf_stage)

10
joj3_config_generator/processers/task.py
View File

@ -1,6 +1,6 @@
import re
jon-lee marked this conversation as resolved
张泊明518370910136 commented 2025-03-03 11:03:41 +08:00
Review
Copy Link

Path should not be relative to JOJ3_CONFIG_ROOT in this file, should be relative to task.toml dir

Path should not be relative to `JOJ3_CONFIG_ROOT` in this file, should be relative to `task.toml` dir
李衍志523370910113 commented 2025-03-03 11:11:32 +08:00
Review
Copy Link

I reckon you said things is relative to JOJ3_CONFIG_ROOT in JTC before. we have a task.type in task.toml to mend the path

I reckon you said things is relative to `JOJ3_CONFIG_ROOT` in JTC before. we have a `task.type` in `task.toml` to mend the path
张泊明518370910136 commented 2025-03-03 11:15:21 +08:00
Review
Copy Link

config.path is relative to JOJ3_CONFIG_ROOT.

`config.path` is relative to `JOJ3_CONFIG_ROOT`.
李衍志523370910113 commented 2025-03-03 13:32:29 +08:00
Review
Copy Link

could you explain further? I m not quite sure my understanding is clear.

could you explain further? I m not quite sure my understanding is clear.
张泊明518370910136 commented 2025-03-04 03:05:54 +08:00
Review
Copy Link

In joj3_config_generator/models/task.py, Config.path is relative to JOJ3_CONFIG_ROOT, so task.toml will located at JOJ3_CONFIG_ROOT / task_conf.path in JTC.

In `joj3_config_generator/models/task.py`, `Config.path` is relative to `JOJ3_CONFIG_ROOT`, so `task.toml` will located at `JOJ3_CONFIG_ROOT / task_conf.path` in JTC.
import shlex
jon-lee marked this conversation as resolved
张泊明518370910136 commented 2025-03-03 11:27:04 +08:00
Review
Copy Link

Some with_.update is still using raw dict, not model with model_dump.

Some `with_.update` is still using raw dict, not model with `model_dump`.
from typing import List, Tuple
from typing import Set
from joj3_config_generator.models import result, task
from joj3_config_generator.models.const import JOJ3_CONFIG_ROOT
@ -33,8 +33,8 @@ def get_conf_stage(
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-03 11:54:43 +08:00
Outdated
Review
Copy Link

should loop through conf_stage.parsers here and update the with field according to the parser name.

should loop through `conf_stage.parsers` here and update the `with` field according to the parser name.
李衍志523370910113 commented 2025-03-03 13:36:23 +08:00
Outdated
Review
Copy Link

I think its already implemented in each of the fix_parsers functions

I think its already implemented in each of the `fix_parsers` functions
张泊明518370910136 commented 2025-03-03 13:43:12 +08:00
Outdated
Review
Copy Link

No, do not find the parser in the fix_xxx function. Instead, iterate through the parsers here and decide how to fill in the with.

No, do not find the parser in the `fix_xxx` function. Instead, iterate through the parsers here and decide how to fill in the `with`.
李衍志523370910113 commented 2025-03-03 16:17:53 +08:00
Outdated
Review
Copy Link

resolved.

resolved.
张泊明518370910136 commented 2025-03-04 03:02:45 +08:00
Outdated
Review
Copy Link

Use a dict to store parser name, field, function to process.

    process_dict: Dict[
        str, Tuple[Callable[[result.ParserConfig, BaseModel], None], BaseModel]
    ] = {
        "clangtidy": (fix_keyword, task_stage.clangtidy),
        "keyword": (fix_keyword, task_stage.keyword),
        "diff": (fix_diff, task_stage.diff),
    }
    for i, parser in enumerate(task_stage.parsers):
        if parser in process_dict:
            func, parser_model = process_dict[parser]
            func(conf_stage.parsers[i], parser_model)
Use a dict to store parser name, field, function to process. ``` process_dict: Dict[ str, Tuple[Callable[[result.ParserConfig, BaseModel], None], BaseModel] ] = { "clangtidy": (fix_keyword, task_stage.clangtidy), "keyword": (fix_keyword, task_stage.keyword), "diff": (fix_diff, task_stage.diff), } for i, parser in enumerate(task_stage.parsers): if parser in process_dict: func, parser_model = process_dict[parser] func(conf_stage.parsers[i], parser_model) ```
李衍志523370910113 commented 2025-03-04 14:46:40 +08:00
Outdated
Review
Copy Link

resolved.

resolved.
def get_executor_with_config(
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-05 03:51:41 +08:00
Outdated
Review
Copy Link

Do we need to support both kinds of names?

Do we need to support both kinds of names?
李衍志523370910113 commented 2025-03-05 08:42:25 +08:00
Outdated
Review
Copy Link

probably yes, since it is easy for new ta to type it wrong

probably yes, since it is easy for new ta to type it wrong
张泊明518370910136 commented 2025-03-05 09:00:28 +08:00
Outdated
Review
Copy Link

parsers name should be a str enum, force them to use the correct names

parsers name should be a str enum, force them to use the correct names
李衍志523370910113 commented 2025-03-05 09:09:46 +08:00
Outdated
Review
Copy Link

ok, then removed.

ok, then removed.
task_stage: task.Stage, cached: List[str]
) -> Tuple[result.ExecutorWith, List[str]]:
task_stage: task.Stage, cached: Set[str]
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-02-23 22:32:35 +08:00
Outdated
Review
Copy Link

underscore

underscore
李衍志523370910113 commented 2025-02-25 11:02:24 +08:00
Outdated
Review
Copy Link

fixed

fixed
) -> result.ExecutorWith:
file_import = task_stage.files.import_
copy_in_files = [file for file in file_import if file not in cached]
file_export = task_stage.files.export
@ -61,8 +61,8 @@ def get_executor_with_config(
)
for file in file_export:
if file not in cached:
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-03 11:25:43 +08:00
Outdated
Review
Copy Link

not necessary

not necessary
李衍志523370910113 commented 2025-03-03 12:51:37 +08:00
Outdated
Review
Copy Link

resolved.

resolved.
cached.append(file)
return (executor_with_config, cached)
cached.add(file)
return executor_with_config
def fix_keyword(

40
tests/convert/basic/task.json
View File

@ -222,11 +222,11 @@
}
},
"copyInCached": {
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/ex2-msan": "h7/build/ex2-msan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json"
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json",
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan"
},
"copyInDir": ".",
"copyOut": [
@ -322,11 +322,11 @@
}
},
"copyInCached": {
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/ex2-msan": "h7/build/ex2-msan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json"
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json",
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan"
},
"copyInDir": ".",
"copyOut": [
@ -444,11 +444,11 @@
"cpuSetLimit": "",
"copyIn": {},
"copyInCached": {
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/ex2-msan": "h7/build/ex2-msan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json"
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json",
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan"
},
"copyInDir": ".",
"copyOut": [
@ -542,11 +542,11 @@
"cpuSetLimit": "",
"copyIn": {},
"copyInCached": {
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/ex2-msan": "h7/build/ex2-msan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json"
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json",
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan"
},
"copyInDir": ".",
"copyOut": [
@ -639,11 +639,11 @@
"cpuSetLimit": "",
"copyIn": {},
"copyInCached": {
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan",
"h7/build/ex2-msan": "h7/build/ex2-msan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json"
"h7/build/ex2-asan": "h7/build/ex2-asan",
"h7/build/compile_commands.json": "h7/build/compile_commands.json",
"h7/build/ex2": "h7/build/ex2",
"h7/build/ex2-ubsan": "h7/build/ex2-ubsan"
},
"copyInDir": ".",
"copyOut": [