Remove CLI options, parse commit msg to decide which conf file to read #14

New Issue

张泊明518370910136 · 2024-03-07T09:23:17+08:00

张泊明518370910136 commented

2024-03-07 09:23:17 +08:00

e.g. with commit msg contains joj. h8, joj3 will read h8.toml and run.

e.g. with commit msg contains `joj. h8`, `joj3` will read `h8.toml` and run.

张泊明518370910136 added this to the (deleted) project 2024-03-07 09:23:33 +08:00

张泊明518370910136 added the

labels 2024-03-07 09:23:45 +08:00

manuel commented

2024-03-07 10:38:31 +08:00

based on "patched" conventional commits:

compilation/CQ: feat, fix, refactor, perf, test, build, revert
OJ (implies compilation/CQ): joj, grading
nothing: doc, style, ci, chores

note: whether or not to run CQ also depends on the course requirements (eg. no need for latex)

Corner cases: PR appear on drone with long complex messages featuring CN char (repo health check let it pass). eg. https://focs.ji.sjtu.edu.cn:2222/engr151-23fa/hteam-21/937

based on "patched" conventional commits: - compilation/CQ: `feat, fix, refactor, perf, test, build, revert` - OJ (implies compilation/CQ): `joj, grading` - nothing: `doc, style, ci, chores` note: whether or not to run CQ also depends on the course requirements (eg. no need for latex) Corner cases: PR appear on drone with long complex messages featuring CN char (repo health check let it pass). eg. https://focs.ji.sjtu.edu.cn:2222/engr151-23fa/hteam-21/937

manuel commented

2024-03-09 09:39:42 +08:00

note: if only relying on commit message, then we need a very "strict" commit message structure and early reject anything failing \to at repo health check (#16)

suggestion: pcc: assignment description where pcc means "patched conventional commit` assignment is the assignment name as defined in the toml config file used for that repo , and description is either a text or numbers for oj exercises to test

eg. feat. hw3 meam leak in ex3 or joj. h4 2 5

downside:

forcing the assignment is mandatory for repos with more than 1 assignment (eg. hw) but doesn't make much sense for a project repo where only that project is completed...
it could even lead to pb if students want to test p1m1 on JOJ without having a p1m1 folder (which we clearly don't want...)

workaround: in the prev implementation we would discover the task (eg. h8 or p1m1 ) and jump inside the corresponding folder (h8) if it existed (so no jump for p1m1).

note: if only relying on commit message, then we need a very "strict" commit message structure and early reject anything failing $\to$ at repo health check (#16) suggestion: `pcc: assignment description` where `pcc` means "patched conventional commit` assignment is the assignment name as defined in the toml config file used for that repo , and description is either a text or numbers for oj exercises to test eg. `feat. hw3 meam leak in ex3` or `joj. h4 2 5` downside: - forcing the assignment is mandatory for repos with more than 1 assignment (eg. hw) but doesn't make much sense for a project repo where only that project is completed... - it could even lead to pb if students want to test p1m1 on JOJ without having a p1m1 folder (which we clearly don't want...) workaround: in the prev implementation we would discover the task (eg. `h8` or `p1m1` ) and jump inside the corresponding folder (`h8`) if it existed (so no jump for `p1m1`).

manuel commented

2024-09-15 10:10:14 +08:00

detection strategy used in 23fa. fix commit format especially for joj submission. for other commits only run repo health + code quality \to correctness makes sense only when a task is ready...

Assume commit message joj: h8 3 5, joj: h8, or joj: p1m1

assignment detection process:

check 1st word A after joj: and take it as the task A to check on joj
if a directory A exists, jump into it and expect to find the code there
if no directory A exists, check if there is an src dir. if yes jump there and find the code
if neither A nor src exist then stay where we are, the code must be here

special case of PR:

format is different, so above strategy cannot apply
search git history for the latest joj commit and use the assignment found there
note: if .git is deleted (as in 23fa) this check should be run very early

detection strategy used in 23fa. fix commit format especially for joj submission. for other commits only run repo health + code quality $\to$ correctness makes sense only when a task is ready... Assume commit message `joj: h8 3 5`, `joj: h8`, or `joj: p1m1` assignment detection process: - check 1st word $A$ after `joj:` and take it as the task $A$ to check on joj - if a directory $A$ exists, jump into it and expect to find the code there - if no directory $A$ exists, check if there is an `src` dir. if yes jump there and find the code - if neither $A$ nor `src` exist then stay where we are, the code must be here special case of PR: - format is different, so above strategy cannot apply - search git history for the latest `joj` commit and use the assignment found there - note: if `.git` is deleted (as in 23fa) this check should be run very early

张泊明518370910136 commented

2024-09-15 19:22:55 +08:00

we may need 3 config files for each type:

only health check and code quality
regular test cases
hidden test cases (need to consider how to trigger these cases)

we may need 3 config files for each type: 1. only health check and code quality 2. regular test cases 3. hidden test cases (need to consider how to trigger these cases)

manuel commented

2024-09-15 19:49:17 +08:00

based on prev courses:

health check (hc) only
hc + compilation only (eg. latex)
hc + code quality (cq) + compilation (but code quality without compilation could make sense for interpreted languages)
hc + (optional compilation) + cq + joj

i think our modular design should make it easy to load any combination of plugin we want. (this is how i made it work last fall)

based on prev courses: - health check (hc) only - hc + compilation only (eg. latex) - hc + code quality (cq) + compilation (but code quality without compilation could make sense for interpreted languages) - hc + (optional compilation) + cq + joj i think our modular design should make it easy to load any combination of plugin we want. (this is how i made it work last fall)

周赵嘉程521432910016 commented

2024-09-19 09:04:47 +08:00

Not sure about current hierachy. What's the path for different config file or there is a rule to name the config file for different usage?

张泊明518370910136 commented

2024-09-19 09:06:14 +08:00

all the config files are in the same dir but have different names

周赵嘉程521432910016 commented

2024-09-19 10:34:20 +08:00

What's current pattern to name the config?

https://focs.ji.sjtu.edu.cn:2222/FOCS-dev/JOJ3/407/1/4

What's current pattern to name the config? https://focs.ji.sjtu.edu.cn:2222/FOCS-dev/JOJ3/407/1/4

张泊明518370910136 commented

2024-09-19 10:41:38 +08:00

Just conf.toml by default if nothing is matched in the test cases.

Just `conf.toml` by default if nothing is matched in the test cases.

周赵嘉程521432910016 commented

2024-09-19 10:44:55 +08:00

It would be bad for students? If we don't provide an error and they made a bad commit msg they may think everything is working "fine"

张泊明518370910136 commented

2024-09-19 10:55:21 +08:00

It depends on the requirements of the teaching team.

周赵嘉程521432910016 commented

2024-09-19 11:08:33 +08:00

So what should I do? Can I just give students some output msg if commit msg fail to match any pattern? ( like joj: bad msg and I just use the conf.toml and output some msg like error: bad pattern for oj you need to follow pattern joj: h[0-9]+?

So what should I do? Can I just give students some output msg if commit msg fail to match any pattern? ( like joj: bad msg and I just use the conf.toml and output some msg like `error: bad pattern for oj you need to follow pattern joj: h[0-9]+`?

manuel commented

2024-09-19 11:10:24 +08:00

It would be bad for students? If we don't provide an error and they made a bad commit msg they may think everything is working "fine"

what do you mean? committing with "h8" instead of "p2" for instance? if this is the case then "most likely" they should realise output is not as expected?

> It would be bad for students? If we don't provide an error and they made a bad commit msg they may think everything is working "fine" what do you mean? committing with "h8" instead of "p2" for instance? if this is the case then "most likely" they should realise output is not as expected?

周赵嘉程521432910016 commented

2024-09-19 11:13:04 +08:00

I mean student may not understand what's going on if their msg is joj: some random words They may think they commit with correct pattern if we don't give they any error.

I mean student may not understand what's going on if their msg is `joj: some random words` They may think they commit with correct pattern if we don't give they any error.

manuel commented

2024-09-19 11:44:41 +08:00

ideally (maybe to be fully implemented later) we only want to allow conventional commit messages with added keyword joj. pattern should be: joj: assignment subassignment where assignment is defined by TT (eg. h2 in my courses but could be hw2 or homework-2 in other courses). if JOJ config file has no entry for the assignment then things should "fail" in some way (eg. "report no such assignment").

\to if subassignment is missing then it means "check all subassignemnts" (joj: h2 means check the whole h2, while joj: h2 4 6 means only check exercises 4 and 6) if an assignment of subassignment doesn't exist then stop work and say that it failed

\to this should be caught early so that we don't do any useless work (eg. try to compile). maybe part of repo check when parsing commit message (we detect whether it is meaningful or not, if not no need to go any further...)

ideally (maybe to be fully implemented later) we only want to allow conventional commit messages with added keyword `joj`. pattern should be: `joj: assignment subassignment` where assignment is defined by TT (eg. `h2` in my courses but could be `hw2` or `homework-2` in other courses). if JOJ config file has no entry for the assignment then things should "fail" in some way (eg. "report no such assignment"). $\to$ if `subassignment` is missing then it means "check all subassignemnts" (`joj: h2` means check the whole h2, while `joj: h2 4 6` means only check exercises 4 and 6) if an assignment of subassignment doesn't exist then stop work and say that it failed $\to$ this should be caught early so that we don't do any useless work (eg. try to compile). maybe part of repo check when parsing commit message (we detect whether it is meaningful or not, if not no need to go any further...)

周赵嘉程521432910016 commented

2024-09-19 12:01:59 +08:00

So shall we give they some hint now if this happen?

Current strategy is just run healthcheck only if following issue happens.

→ this should be caught early so that we don't do any useless work (eg. try to compile). maybe part of repo check when parsing commit message (we detect whether it is meaningful or not, if not no need to go any further...)

So shall we give they some hint now if this happen? Current strategy is just run healthcheck only if following issue happens. > → this should be caught early so that we don't do any useless work (eg. try to compile). maybe part of repo check when parsing commit message (we detect whether it is meaningful or not, if not no need to go any further...)

manuel commented

2024-09-19 12:15:31 +08:00

Current strategy is just run healthcheck only if following issue happens.

healthcheck should be run on any commit? then "prepare the way" for JOJ. fialed healthcheck should prevent JOJ running (otherwise nobody will care... until they get their grade, and they will complain that they didn't see any problem reported about their submission)

> Current strategy is just run healthcheck only if following issue happens. healthcheck should be run on any commit? then "prepare the way" for JOJ. fialed healthcheck should prevent JOJ running (otherwise nobody will care... until they get their grade, and they will complain that they didn't see any problem reported about their submission)

周赵嘉程521432910016 commented

2024-09-19 18:16:36 +08:00

Already implemented through force quit.

healthcheck should be run on any commit? then "prepare the way" for JOJ. fialed healthcheck should prevent JOJ running (otherwise nobody will care... until they get their grade, and they will complain that they didn't see any issue)

Already implemented through force quit. > healthcheck should be run on any commit? then "prepare the way" for JOJ. fialed healthcheck should prevent JOJ running (otherwise nobody will care... until they get their grade, and they will complain that they didn't see any issue)

周赵嘉程521432910016 commented

2024-09-23 12:46:25 +08:00

We may need jobtype flag in the config for each hw s.t. we know which stages to perform after parsing the config. At this stage I can only parse based on the name of stage which is not a wise choice I think.

73f1688e41/cmd/joj3/conf.go (L76-L117)

We may need jobtype flag in the config for each hw s.t. we know which stages to perform after parsing the config. At this stage I can only parse based on the name of stage which is not a wise choice I think. https://focs.ji.sjtu.edu.cn/git/FOCS-dev/JOJ3/src/commit/73f1688e419258406628a74bb367497c2aff4657/cmd/joj3/conf.go#L76-L117

张泊明518370910136 commented

2024-09-26 15:46:15 +08:00

Done by meta conf.

张泊明518370910136 closed this issue

2024-09-26 15:46:15 +08:00

Sign in to join this conversation.