Skip to content

Clarify usage of other_locations key #57

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 3 commits into from
Jun 6, 2017
Merged

Clarify usage of other_locations key #57

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b4a2591
Clarify usage of other_locations key
Jun 6, 2017
ac8d6aa
fix grammar
Jun 6, 2017
02dcf1f
remove comma
Jun 6, 2017
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
22 changes: 20 additions & 2 deletions SPEC.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ We welcome your participation and appreciate your patience as we finalize the pl
- [Remediation points](#remediation-points)
- [Locations](#locations)
- [Positions](#positions)
- [Other Locations](#other-locations)
- [Contents](#contents)
- [Source code traces](#source-code-traces)
- [Packaging](#packaging)
Expand Down Expand Up @@ -201,7 +202,7 @@ The baseline remediation points value is 50,000, which is the time it takes to f

### Locations

Locations refer to ranges of a source code file. A Location contains a `path`, a source range, (expressed as `lines` or `positions`), and an optional array of `other_locations`. Here's an example location:
Locations refer to ranges of a source code file. A Location contains a `path` and a source range, (expressed as `lines` or `positions`). Here's an example location:
Copy link
Contributor

Choose a reason for hiding this comment

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

I still don't think the comma after "range" here is grammatical: it makes it read as though the parenthetical applies to "a path and a source range", but in fact it's only referring to "a source range".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops left that by accident. thanks


```json
{
Expand Down Expand Up @@ -266,6 +267,23 @@ line of the file.

Offsets, however are 0-based. A Position of `{ "offset": 4 }` represents the _fifth_ character in the file. Importantly, the `offset` is from the beginning of the file, not the beginning of a line. Newline characters (and all characters) count when computing an offset.

### Other Locations

Other locations is an optional array of [Location](#locations) objects:

```json
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I don't think a full example here is informative or necessary. It's just an array of an object type described elsewhere, and the full Issue schema makes that clear.

If you think it's important to keep, though, please make the formatting consistent with other cases that describe a top-level key, e.g. the example in the Trace section. (Specifically, I'm referring to not having the wrapping {} around the whole thing, which make it seem like this whole snippet is a valid object, which it isn't in our schema.)

"other_locations": [
{
"path": "foo.rb",
"lines": { "begin": 25, "end": 55 }
},
{
"path": "bar.rb",
"lines": { "begin": 20, "end": 50 }
}
]
```

### Contents

Content gives more information about the issue's check, including a description of the issue, how to fix it, and relevant links. They are expressed as a hash with a `body` key. The value of this key should be a [Markdown](http://daringfireball.net/projects/markdown/) document. For example:
Expand All @@ -279,7 +297,7 @@ Content gives more information about the issue's check, including a description

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.
* `locations` -- **[Location] - Required**. An array of [Location](#locations) objects.
* `stacktrace` -- **Boolean - Optional **. *Default: false* When `true`, this `Trace` object will be treated like an ordered stacktrace by the CLI and the Code Climate UI.

An example trace:
Expand Down