Skip to content

Conversation

@ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Mar 29, 2019

This sorts api/inheritance.json, just like in mdn/kumascript#709.

Note that I kept inherits preceding implements.


review?(@a2sheppy, @atopal, @chrisdavidmills, @Elchi3, @teoli2003, @wbamberg)

@ExE-Boss ExE-Boss force-pushed the api/inheritance/sort branch from 2aca479 to ebf1e29 Compare March 29, 2019 01:36
@chrisdavidmills
Copy link
Contributor

This is way too big for me to review; any chance you can break it down into smaller bits of work? Per API might make sense.

@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented Feb 17, 2020

@chrisdavidmills I only sorted the file.

You can verify this yourself by running:

const assert = require("assert");

// The `api/inheritance.json` on the master branch:
const apiInhMaster = require("./api/inheritance.master.json");

// The `api/inheritance.json` from this PR:
const apiInhPR358 = require("./api/inheritance.pr358.json");

assert.deepStrictEqual(
	Reflect.ownKeys(apiInhPR358),
	Reflect.ownKeys(apiInhMaster).sort(),
);
assert.deepStrictEqual(
	Reflect.ownKeys(apiInhPR358).map(k => apiInhPR358[k].inherits),
	Reflect.ownKeys(apiInhMaster).sort().map(k => apiInhPR358[k].inherits),
);
assert.deepStrictEqual(
	Reflect.ownKeys(apiInhPR358).map(k => apiInhPR358[k].implements),
	Reflect.ownKeys(apiInhMaster).sort().map(k => apiInhPR358[k].implements.sort()),
);

Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

@ExE-Boss ah, I get it now ;-)

I did a quick check on this using both a JSON validator and the command line comm tool to make sure all original lines are still present, and things are looking good.

@chrisdavidmills chrisdavidmills merged commit 1f35d75 into mdn:master Feb 19, 2020
@ExE-Boss ExE-Boss deleted the api/inheritance/sort branch February 19, 2020 14:04
@Elchi3 Elchi3 removed request for Elchi3 and wbamberg March 24, 2020 17:27
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.

2 participants