ioccc-src / mkiocccentry

Form an IOCCC entry as a compressed tarball file
Other
28 stars 5 forks source link

Improve rebuilding txzchk test error files #858

Closed xexyl closed 2 months ago

xexyl commented 2 months ago

The script test_ioccc/test_txzchk.sh now has a new option: -B. This rebuilds the test error files. The script prompts the user to verify that everything is correct. If it does not get a YES (case sensitive!) it will exit 3 (invalid command line). If this is the case the rest of the rule is not run so no notice about adding and committing the new or updated files.

The rule in the Makefile now is called (the previous one still exists though): rebuild_txzchk_test_errors.

The test_ioccc/test_txzchk/README.md file has been updated for this (there was also an error in it but that text has been removed as the make rule tells the user what to do which is what the incorrect text was only of course the text is now corrected).

xexyl commented 2 months ago

It seems like the script is a good place to do it esp as it now can prompt the user to make sure it is okay to do it. The comment in the Makefile is still there of course but the script having control seems like a good idea. There might be an argument to not have this but I don't know: I think centralising it is useful and again it allows one to control the user a bit more to make sure that it's not done without manual inspection. Imagine if one were to just run the rule in the top level directory without thinking about it, for example.

The script will normally just run the tests. I wonder if the script should (with the -B flag) tell the user what to do though. Meaning run the test script again (without the -B flag) to make sure it's okay and then add, commit and if necessary open a pull request. Then the Makefile could just run the script with -B. What do you think?

I will do it later if you think it should change.

xexyl commented 2 months ago

I also noticed something: these test scripts do not have man pages. Would you like me to add these? I presume the priority is pretty low as it would delay work in the other repo.

I will look at the other comments here and in the other repo later on today .. I hope. if not then tomorrow I should be able to. For now though it's another break. Two days awake too early and I won't be able to rest again but hopefully will be a fine day.

Hope you're sleeping well / had a nice sleep!

lcn2 commented 2 months ago

I also noticed something: these test scripts do not have man pages. Would you like me to add these? I presume the priority is pretty low as it would delay work in the other repo.

Low priority add, yes. Good idea @xexyl

I will look at the other comments here and in the other repo later on today .. I hope. if not then tomorrow I should be able to. For now though it's another break. Two days awake too early and I won't be able to rest again but hopefully will be a fine day.

Hope you're sleeping well / had a nice sleep!

Well thank you, hope you had one as well.

xexyl commented 2 months ago

I also noticed something: these test scripts do not have man pages. Would you like me to add these? I presume the priority is pretty low as it would delay work in the other repo.

Low priority add, yes. Good idea @xexyl

Figured it would be low priority.

I will look at the other comments here and in the other repo later on today .. I hope. if not then tomorrow I should be able to. For now though it's another break. Two days awake too early and I won't be able to rest again but hopefully will be a fine day.

Hope you're sleeping well / had a nice sleep!

Well thank you, hope you had one as well.

Woke up too early again but I am okay at least.

lcn2 commented 2 months ago

Thanks @xexyl .. and it looks like the so-called GitHub checks are working again.

xexyl commented 2 months ago

Thanks @xexyl ..

Welcome.

and it looks like the so-called GitHub checks are working again.

... that's good!