EVA-470 Summary report independent from checks #52
Conversation
| friend class odb::access; | ||
| NoMetaDefinitionError() {} | ||
| public: | ||
| NoMetaDefinitionError(size_t line, std::string message, std::string column, std::string field) |
There was a problem hiding this comment.
Other classes in the hierarchy use const std::string & as string arguments for the constructor.
| public: | ||
| virtual void write_error(Error &error) | ||
| { | ||
| std::cout << error.what() << std::endl; |
There was a problem hiding this comment.
This will conflict with the plain text report, because they both write to stdout: https://github.com/jmmut/vcf-validator/blob/76ad342fc2cb16af51aa2d91fc464a7bd1354060/inc/vcf/report_writer.hpp#L42
There was a problem hiding this comment.
complete report removed, so now only the summary report writes to stdout
jmmut
force-pushed
the
feature/repeated-report
branch
from
22be434 to
fc73153
Compare
| #pragma db object | ||
| struct NoMetaDefinitionError : public Error | ||
| { | ||
| private: |
There was a problem hiding this comment.
Could the private block be moved to the end of the class declaration, as in Error?
| bool is_already_reported(std::string const &meta_type, std::string const &id) const | ||
| { | ||
| typedef std::multimap<std::string,std::string>::const_iterator iter; | ||
| std::pair<iter, iter> range = undefined_metadata.equal_range(meta_type); |
There was a problem hiding this comment.
undefined_metadata should be renamed to reflect is stores already reported errors, as the methods already do properly.
| class SummaryReportWriter : public ReportWriter | ||
| { | ||
| public: | ||
| SummaryReportWriter(std::ostream &out) : out(out) {} |
There was a problem hiding this comment.
reverted, it breaks gcc 4.8 compilation
| @@ -196,7 +197,7 @@ namespace | |||
| } | |||
| outputs.emplace_back(new ebi::vcf::OdbReportRW(db_filename)); | |||
| } else if (out == "stdout") { | |||
There was a problem hiding this comment.
We could call the report types "complete" / "full" (I think "full" is the most frequent term) and "summary", instead of "database" and "stdout".
There was a problem hiding this comment.
the problem with that is that the reports are written in different ways, the database one only writes to a file, while the stdout only writes to stdout, unless you redirect manually to a file. I we change the name to "full" and "summary" I would change the behaviour of "summary" to not write to stdout, but directly to a file, just as "full" does currently.
There was a problem hiding this comment.
Agreed to think a bit more about this and make a decision in the short term.
| @@ -33,6 +33,8 @@ namespace ebi | |||
| return std::shared_ptr<Error>(new HeaderSectionError{line, message}); | |||
| case ErrorCode::body_section: | |||
| return std::shared_ptr<Error>(new BodySectionError{line, message}); | |||
| case ErrorCode::no_meta_definition: | |||
There was a problem hiding this comment.
For another PR, but this get_error_instance function is only used by the SqliteReport so we should get rid of it.
jmmut
force-pushed
the
feature/repeated-report
branch
from
6fd8999 to
8ca88b6
Compare
Allow filtering superfluous warnings when writing to standard output.
The checks are always done and always raised, but only reported in non-summary reports.
The previous behaviour was that if the error was already raised, it was not raised for any report type.