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.

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

Allow specs to be represented by a simple string #3

Merged
merged 4 commits into from
Jan 31, 2020
Merged

Allow specs to be represented by a simple string #3

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b0e105c
Allow specs to be represented by a simple string
tidoust Jan 21, 2020
6f77ab1
Simplify string detection logic
tidoust Jan 21, 2020
82aeffd
Add JSON Schema and schema validation to linting logic
tidoust Jan 23, 2020
55acd74
Add tests for linter
tidoust Jan 24, 2020
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
14 changes: 13 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,16 @@ This list is meant to be an up-to-date input source for projects that run analys

## Format

`specs.json` contains an array of objects, each with a single property `url`.
`specs.json` contains an array of items that are either:
1. a string that represents a valid URL;
2. an object with a `url` property (additional properties to be added over time as needed)

In most cases, a URL should be enough. As such, to keep the file readable by human beings, the string format should be preferred whenever possible. The object format should only be used when additional properties need to be specified.

Tools that want to ingest the list are encouraged to use the `specs` array exported by the `index.js` file, which contains a normalized version of the list in `specs.json` where each entry is an object with a `url` property to ease processing.

## Linting

Run `node lint` to identify potential linting issues (automatically done for pull requests).

Run `node lint --fix` to overwrite `specs.json` locally with the linted version.
6 changes: 5 additions & 1 deletion index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
"use strict";

const specs = require("./specs.json");
const isString = obj =>
Object.prototype.toString.call(obj) === "[object String]";

const specs = require("./specs.json")
.map(spec => isString(spec) ? { url: spec } : spec);
tidoust marked this conversation as resolved.
Show resolved Hide resolved

module.exports = { specs };
22 changes: 13 additions & 9 deletions lint.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,23 @@

const fs = require("fs").promises;

const isString = obj =>
tidoust marked this conversation as resolved.
Show resolved Hide resolved
Object.prototype.toString.call(obj) === "[object String]";

// Lint by normalizing specs.json and comparing it to the original,
// fixing it in place if |fix| is true.
async function lint(fix = false) {
const specsBuffer = await fs.readFile("./specs.json");
let specs = JSON.parse(specsBuffer);

const fixed = specs.map(spec => {
const url = new URL(spec.url).toString();
return { url };
});
fixed.sort((a, b) => {
a.url.localeCompare(b.url);
});
const specs = JSON.parse(specsBuffer);

const sorted = specs
.map(spec => isString(spec) ? { url: spec } : spec)
.map(spec => Object.assign({}, spec, { url: new URL(spec.url).toString() }));
Copy link
Member

Choose a reason for hiding this comment

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

This allows for arbitrary additional properties, most likely in the form of typos if we add more properties. I'd prefer this code to normalize everything that is known and drop anything else. Since currently no additional properties are allowed, just require all entries to be a string.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was assuming that we would in any case add some kind of JSON schema to validate the structure before we pass it to the linter. I'll add some code logic to that effect.

Copy link
Member

Choose a reason for hiding this comment

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

I started out using ajv myself, but ripped it out since the schema was so trivial. Now when either a string or object are allowed it's less trivial, so I'm happy with using it.

I'll have a few days off for the lunar new year, but will review this when I'm back next week!

Copy link
Member Author

Choose a reason for hiding this comment

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

I just added a JSON Schema and schema validation. The schema is a bit dumb for now since it allows an object with a URL, but then that's useless as long as we don't have additional properties, especially since the linting process will convert objects to URL strings. At least we'll have the right skeleton and logic in place for when we add properties.

sorted.sort((a, b) => a.url.localeCompare(b.url));

// Prefer URL-only format when we only have a URL
const fixed = sorted
.map(spec => (Object.keys(spec).length > 1) ? spec : spec.url);

const fixedBuffer = Buffer.from(JSON.stringify(fixed, null, " ") + "\n");
if (!specsBuffer.equals(fixedBuffer)) {
Expand Down
4 changes: 1 addition & 3 deletions specs.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
[
{
"url": "https://compat.spec.whatwg.org/"
}
"https://compat.spec.whatwg.org/"
]