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

Improve error handling on Analysis page #97

Merged
merged 2 commits into from
May 30, 2017
Merged

Conversation

bradleyfalzon
Copy link
Owner

DB StartAnalysis should take commitFrom, commitTo, and requestNumber:

Previously these fields were only added after a successful analysis.
This change makes those three required fields a part of the constructor
so no matter what failure scenario there is, those fields are still set.

The other option is to ensure a failed analysis still sets some of
these fields, but there may be some scenarios where those fields may
have been missed, such as a panic.

Analysis page fetching diff should not be a critical error:

The db.Analysis type contains a list of the issues per tool that were
stored in the database. We use these in two ways:

A) Provide it to web.DiffIssues function to combine a diff/patch with
the issue to show the issue inline.

B) Show a summary of the issues, grouped by tool.

If fetching the diff/patch fails, we can still show the issues per
tool, this contains all the information a user really requires.

Now, patches is only set in the template if there were issues *and*
there was no errors fetching the diff.

Relates to #88.

Previously these fields were only added after a successful analysis.
This change makes those three required fields a part of the constructor
so no matter what failure scenario there is, those fields are still set.

The other option is to ensure a failed analysis still sets some of
these fields, but there may be some scenarios where those fields may
have been missed, such as a panic.

Relates to #88.
The db.Analysis type contains a list of the issues per tool that were
stored in the database. We use these in two ways:

A) Provide it to web.DiffIssues function to combine a diff/patch with
the issue to show the issue inline.

B) Show a summary of the issues, grouped by tool.

If fetching the diff/patch fails, we can still show the issues per
tool, this contains all the information a user really requires.

Now, patches is only set in the template if there were issues *and*
there was no errors fetching the diff.

Relates to #88.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 60.278% when pulling 7c6ccfe on error-handling into 837835d on master.

@bradleyfalzon bradleyfalzon merged commit 9e35dc9 into master May 30, 2017
@bradleyfalzon bradleyfalzon deleted the error-handling branch May 30, 2017 10:22
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.

None yet

2 participants