Skip to content

Add trace #20

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 2, 2015
Merged

Add trace #20

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions SPEC.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ An `issue` represents a single instance of a real or potential code problem, det
* `content` -- **Optional**. A markdown snippet describing the issue, including deeper explanations and links to other resources.
* `categories` -- **Required**. At least one category indicating the nature of the issue being reported.
* `location` -- **Required**. A `Location` object representing the place in the source code where the issue was discovered.
* `other_locations` -- **Optional.** An array of `Location` objects useful for engines which highlight more than one source location in an issue.
* `trace` -- **Optional.** A `Trace` object representing other interesting source code locations related to this issue.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have engines built on top of other_locations yet? Do we need to flesh out versioning and compatability concerns before this can move forward?

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought about this too and I think we're fine, since we can update codeclimate and builder to handle both sorts of output.

It gets tricky the other direction, because it's hard for us to force updates to all engines. But this direction shouldn't be too much work.

(Though handling versioning would be nice)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fhwang @pbrisbin The only one is duplication, which is not fully released.

* `remediation_points` -- **Optional**. An integer indicating a rough estimate of how long it would take to resolve the reported issue.
* `severity` -- **Optional**. A `Severity` string (`info`, `normal`, or `critical`) describing the potential impact of the issue found.

Expand Down Expand Up @@ -180,9 +180,35 @@ Contents give more information about the issue's check, including a description
"body": "This cop checks that the ABC size of methods is not higher than the configured maximum. The ABC size is based on assignments, branches (method calls), and conditions. See [this page](http://c2.com/cgi/wiki?AbcMetric) for more information on ABC size."
}
```
### Other Locations
### Source Code Traces

Some engines require the ability to refer to other source locations. For this reason, the Issue type has an optional `other_locations` field, which is an array of other `Location` items that this issue needs to refer to.
Some engines require the ability to refer to other source locations in describing an issue. For this reason, an `Issue` object can have an associated `Trace`, a data structure meant to represent ordered or unordered lists of source code locations. A `Trace` has the following fields:

* `locations` -- **[Location] - Required**. An array of `Location` objects.
* `stacktrace` -- **Boolean - Optional**. When `true`, this `Trace` object will be treated like an ordered stacktrace by the CLI and the Code Climate UI.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to specify what order the stacktrace is to be interpreted in? Python outputs stack traces in reverse order from most languages, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pbrisbin When you say that a trace is a stacktrace, we'll treat it as an ordered Array, so it's up to the engine to provide it in the order that it should be displayed.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, so if some Python engine shows it one way and some Ruby engine another, we don't care to normalize that, whatever it is it is? Seems fine to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pbrisbin Yes I think that's correct.


An example trace:

```json
"trace": {
"locations": [{
"path": "path/to/file.css",
"lines": {
"begin": 13,
"end": 14
}
},
{
"path": "path/to/file.css",
"lines": {
"begin": 19,
"end": 20
}
}],
"stacktrace": true
}

```

## Versioning

Expand Down