WIP Feature/emit processor #313
Conversation
| public void Register(ICodeSubmissionProcessor processor) | ||
| public void Add(ICodeSubmissionProcessor processor) | ||
| { | ||
| _processors[processor.Command] = processor; |
There was a problem hiding this comment.
Add seems like a misleading name if this can also replace an existing processor.
There was a problem hiding this comment.
changed to add so we throw exceptions when overriding
There was a problem hiding this comment.
This still obscure the intention. A more honest name would be something like SetProcessorForCommand.
|
|
||
| return new SubmitCode(string.Join("\n", unhandledLines), codeSubmission.Id, codeSubmission.ParentId); | ||
| } | ||
| catch (Exception e) |
There was a problem hiding this comment.
This seems overly broad. We should ideally only catch exceptions that we understand, as this might otherwise mask some of our own bugs.
There was a problem hiding this comment.
that is wrapping all exceptions so we capture the exception and the code submission
| var renderer = FindRenderer(source.GetType()); | ||
| return renderer.Render(source, this); | ||
| } | ||
| catch (Exception e) |
There was a problem hiding this comment.
Is there a specific reason (e.g. expected exception type) to do this? catch Exception is something that should typically only ever happen at a much higher level in the code.
There was a problem hiding this comment.
this one is wrapping as well, and capturing the evaluation result
|
|
||
| var state = getScriptState(); | ||
|
|
||
| var codeFile = new FileInfo(Path.Combine(options.Output.FullName, "code.cs")); |
There was a problem hiding this comment.
This should be made to work against IDirectoryAccessor from the outset.
| { | ||
| public SubmitCode CodeSubmission { get; } | ||
|
|
||
| public CodeSubmissionProcessorException(Exception exception, SubmitCode codeSubmission) : base("CodeSubmission processing failed", exception) |
There was a problem hiding this comment.
It would also be helpful to refer to the processor that failed.
d556439 to
238d7ec
Compare
No description provided.