-
Notifications
You must be signed in to change notification settings - Fork 24
Legacy export #441
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.
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
Legacy export #441
Conversation
@mpsijm I guess this can be merged as well? |
Didn't immediately have the time to try it out, but I did now 😛 Is there a reason to not include the input validators? They are part of the spec, and they already were in legacy, now that I took another look. I don't know why they were never included 🤔 We could also include the And if we want to be complete, I don't see why we can't also include the answer validators now. They're a BAPCtools-specific extension, but the spec doesn't forbid including extra files 😇 |
I don't know why... probably because this zip was only ever used as import for domjudge/kattis? |
DOMjudge doesn't run the input validators, but Kattis does. I guess DOMjudge is free to choose this, but it also doesn't hurt to include the validators for archival completeness. @RagnarGrootKoerkamp do you know why the input validators are not in the zip currently? |
(for kattis we do export them) |
Yeah, what @mzuenni said. It probably was domjudge-first and we simply never bothered/realized it. |
Fair enough! I think we can simply export everything then, no reason to leave anything out (tools that don't know how to use some directories, can simply ignore them) 🙂 |
@mpsijm do you have time to take a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made two small documentation fixes, looking good to me 😄 At least, it works for me on the English-only problems from last year 😛
Ready to squash-and-merge? 🙂
Some tests here would be good? I am really unsure if this works as we want... mostly because I have no idea what we want since I have never used these features ^^' @thorehusfeldt do you have multi-language problems/contest to test that nothing breaks and behaves as expected? |
Apparently, `bt validate` also doesn't require them?
I've added some tests verifying the current behaviour (which I think is the correct behaviour). Basically, the test compares the list of files in the resulting ZIP with some hand-crafted list of files, to see if the contents match 🙂 Also, I've renamed If you (and the CI) agree with the tests that I added, this can be squashed-and-merged 😄 |
* make legacy export an explicit command * [export] Add all the directories! * [export] Legacy: remove solution/ and problem_slide/ from export dir * [export] Make answer_validators/ not required Apparently, `bt validate` also doesn't require them? * prepend problem name * fix test * handle languages * use ngerman * add german to wsl * keep languages in sync * keep languages in sync * [export] Add comment explaining why name in problems.yaml can also be str * [doc] Improve singular/plural in explanation of `--languages` * [export][latex] Rename --languages flag to --lang * [test][export] Add assertions for which PDFs should be in the ZIPs * [test] TestContest.test_zip: also remove constituent zip files after test completes --------- Co-authored-by: Maarten Sijm <[email protected]>
No description provided.