Skip to content
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

Don't rerun checkers needlessly #24

Merged
merged 4 commits into from
Apr 18, 2022
Merged

Don't rerun checkers needlessly #24

merged 4 commits into from
Apr 18, 2022

Conversation

fagu
Copy link
Member

@fagu fagu commented Apr 16, 2022

This should fix #20 (comment) using solution proposal 1.

It only allows checkers written in C++, but I guess that was already assumed before. (?)

@t-lenz Please let me know if this looks okay.

Previously, the file 'checker-log.txt' was overwritten each time
a checker was run. That meant that the checker often needed to be
rerun needlessly (as the file was lost).

We now simply capture the standard output and save it in the
'.rules' directory.

We only allow CPPPrograms as checkers.
Copy link
Member

@t-lenz t-lenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good on the checker/constraints side. Only a couple of very minor points:

  1. There's a typo in TaskConfig.py (see the respective comment)
  2. You should update the dates in the “copyright” section of the respective files
  3. We should mention in the docs that checkers (a) must be written in C++ and (b) must not write anything to standard output.

Maybe also someone else who's familiar with the rules system should have a look at this, for example @magula.

try:
checker_log = "checker-log.txt"
if not isinstance(checker, CPPProgram):
raise Exception("Only checkers written in c++ are allowed.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: C++ (with a capitcal C)

@fagu
Copy link
Member Author

fagu commented Apr 17, 2022

Thanks! I've fixed the issues you pointed out.

@magula magula self-requested a review April 18, 2022 12:33
@t-lenz t-lenz merged commit 31a40b2 into main Apr 18, 2022
@chuyang-wang-dev chuyang-wang-dev deleted the checker-log branch January 18, 2025 09:23
@chuyang-wang-dev chuyang-wang-dev restored the checker-log branch January 18, 2025 09:23
@chuyang-wang-dev chuyang-wang-dev deleted the checker-log branch January 18, 2025 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants