-
Notifications
You must be signed in to change notification settings - Fork 16
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
Reference client kick-off #1
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
node_modules | ||
.idea | ||
.vs | ||
bin |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
module.exports = function(config) { | ||
config.set({ | ||
basePath: "", | ||
frameworks: ["jasmine", "sinon", "jasmine-sinon", "source-map-support", "karma-typescript"], | ||
files: [ | ||
{ pattern: "src/**/*.ts", included: true }, | ||
{ pattern: "testing/**/*.ts", included: true }, | ||
{ pattern: "tests/**/*.spec.ts", included: true } | ||
], | ||
exclude: [ | ||
"jsonld-request" | ||
], | ||
preprocessors: { | ||
"**/*.ts": ["karma-typescript"] | ||
}, | ||
karmaTypescriptConfig: { | ||
tsconfig: "tsconfig.json", | ||
coverageOptions: { | ||
instrumentation: false | ||
}, | ||
bundlerOptions: { | ||
entrypoints: /\.spec\.ts$/, | ||
exclude: [ | ||
"jsonld-request", | ||
"pkginfo" | ||
] | ||
} | ||
}, | ||
reporters: ["progress", "karma-typescript"], | ||
port: 9876, | ||
colors: true, | ||
logLevel: config.LOG_INFO, | ||
autoWatch: true, | ||
browsers: ["Chrome"], | ||
singleRun: false, | ||
concurrency: Infinity | ||
}) | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
{ | ||
"name": "hydraclient.js", | ||
"version": "0.0.1", | ||
"scripts": { | ||
"test": "node node_modules/karma/bin/karma start", | ||
"test-once": "node node_modules/karma/bin/karma start --single-run" | ||
}, | ||
"dependencies": { | ||
"jsonld": "^0.4.12", | ||
"typescript": "^2.3.3", | ||
"whatwg-fetch": "^2.0.3" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've got someone asking in "my" heracles to use isomorphic-fetch so that the client can be used both in browser and Node. Let's do that from get-go. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced with isomorphic-fetch |
||
}, | ||
"devDependencies": { | ||
"@types/jasmine": "^2.5.47", | ||
"@types/node": "^7.0.21", | ||
"@types/sinon": "^2.2.2", | ||
"jasmine": "^2.6.0", | ||
"jasmine-sinon": "^0.4.0", | ||
"karma": "^1.7.0", | ||
"karma-chrome-launcher": "^2.1.1", | ||
"karma-jasmine": "^1.1.0", | ||
"karma-jasmine-sinon": "^1.0.4", | ||
"karma-sinon": "^1.0.5", | ||
"karma-source-map-support": "^1.2.0", | ||
"karma-typescript": "^3.0.2", | ||
"sinon": "^2.2.0" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import {IApiDocumentation} from "./DataModel/IApiDocumentation"; | ||
import {IData} from "./DataModel/IData"; | ||
import {IClass} from "./DataModel/IClass"; | ||
import {IOperation} from "./DataModel/IOperation"; | ||
import HydraClient from "./HydraClient"; | ||
|
||
export default class ApiDocumentation implements IApiDocumentation | ||
{ | ||
public supportedClasses: Array<IClass>; | ||
public supportedOperations: Array<IOperation>; | ||
public client: HydraClient; | ||
public entryPoint: string; | ||
|
||
public async getEntryPoint(): Promise<IData> | ||
{ | ||
let response = await window.fetch(this.entryPoint); | ||
if (response.status !== 200) | ||
{ | ||
throw new Error(HydraClient.invalidResponse + response.status); | ||
} | ||
|
||
let metadataProvider = this.client.getMetadataProvider(response); | ||
if (!metadataProvider) | ||
{ | ||
throw new Error(HydraClient.responseFormatNotSupported); | ||
} | ||
|
||
return await metadataProvider.parse(response, true); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import {IMetadataDescription} from "./IMetadataDescription"; | ||
import {IData} from "./IData"; | ||
|
||
export interface IApiDocumentation extends IMetadataDescription | ||
{ | ||
entryPoint: string; | ||
|
||
getEntryPoint(): Promise<IData>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like how this is the functionality of the documentation. I treat the What would you think about smth like IHydraApi api = new HydraApi("http://entrypoint");
IApiDocumentation docs = api.getApidDocumentation();
IResource entry = api.getEntryPoint(); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As for the Single Responsibility Principle - I don't think it would be a violation - It's a functional inheritance and actual implementation could be a simple wrapper on top of an actual implementation. Indeed there is an inconsistency in use cases - some places uses method chaining, i.e.: var operation = client.get("http://example.com")
.getApiDocumentation()
.getEntryPoint() And some other are using client as an invocation facility for operations, i.e.: var operation = client.get("http://example.com")
.getApiDocumentation()
.getEntryPoint()
.getOperations()
.filter(op => op.method == "POST" && op.expects == "Event")[0];
client.invoke(operation, event); I'm not sure which one is better. The latter seems cleaner, but it would force client to do quite a lot (i.e. make distinction on how to work with simple Url string, Link, Iri template or an operation). As for your example, I wouldn't like to see developers create instances - I'd prefer our API to do it on it's own so developers don't need to bother about real implementation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Funny you'd say that. Previously I didn't actually implement it that way ;). Now that I look again at your code, maybe I was misjudging it a little. My actual point is that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's why I'm torn apart - now I somehow tend to stick to that latter approach where client does all the stuff - I'm just worried that the client would be overloaded with logic. var api = client.get("http://example.com");
var apiDocumentation = client.get(api.getApiDocumentationUrl());
var entryPoint = client.get(apiDocumentation.entryPoint);
var operation = entryPoint
.filter(op => op.method == "POST" && op.expects == "Event")[0];
client.invoke(operation, event); looks a bit messy There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, so procedural. Definitely not what I had in mind ;). But read again and notice that I meant moving the implementation details from the function getEntryPoint() {
return this.client.getResource(this.entryPoint);
} Otherwise the snippets in you comment above look good to me. Well, maybe I'd make the operation more self-aware, but that's a different discussion
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, not so different. For simple Urls we'll need to call client's method, but for strongly typed resources we will use their methods - it's not really consistent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From my experience in implementing hypermedia clients with Equivalently, I think it's wise to make There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'ApiDocumentation; is now light and depends on a newly introduced |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import {IResource} from "./IResource"; | ||
|
||
export interface IClass extends IResource | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this used for? To represent There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, someday when we'll reach that point. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, I don't think the code needs (or even should) match the representations on the wire 1:1 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the wire - indeed. But I think client's model should be rooted in hydra's model somehow. |
||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import {IMetadata} from "./IMetadata"; | ||
|
||
export interface IData extends Object | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like this and For There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By IData i meant raw data. The thing that actually client wanted. Metadata - hypermedia controls, data structure description is something that helps a client to understand what it can/needs to do. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think the client wants resource representations. What if instead And metadata? I think I like the idea. You wat to separate the controls from the actual representation, right? A place to put operations, etc? So what if it was called var resource = client.getResource('http://example.com');
resource.controls.getApiDocumentation();
resource.controls.getOperations();
resource.controls.getLinks(); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed I'd love to have raw data and hydra stuff somehow separated. As for the IData replacements - IResource is already used to denote actual resource. I wouldn't like to add a metadata property there. Bu I see direction your thoughts are going - I need to give it some thoughs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @asbjornu thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't find this that much of an issue. Sure, you get stuff mixed a little but that's why these are representations and not resources. Hypermedia is part of the representation, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This library doesn't know what higher level libraries, apps, UIs want to do with the data so I wouldn't introduce such (leaky) separations/classifications here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that this separation might not come easily. I agree that library doesn't know what higher level libraries, apps and UIs want to do with the data. I still think that client should know what is a hypermedia control - it's supposed to consume those. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bundling hypermedia controls into a property seems like a clean approach. 👍 But what are they, exactly? Affordances? Operations? Actions? Controls? Naming things is hard. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed with |
||
{ | ||
metadata: Array<IMetadata>; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import HydraClient from "../HydraClient"; | ||
|
||
export interface IMetadata | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd get rid of this class for the reasons mentioned above There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one is still here, but I got rid of IMetadataDescriptor. This one also got renamed. |
||
{ | ||
client: HydraClient; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import {IMetadata} from "./IMetadata"; | ||
import {IClass} from "./IClass"; | ||
import {IOperation} from "./IOperation"; | ||
import HydraClient from "../HydraClient"; | ||
|
||
export interface IMetadataDescription extends IMetadata | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly to the above. Hydra specification doesn't speak about any metadata anywhere. The properties below suggest to me that maybe this would actually be the I'm sorry I contradict earlier comments but I'm coming up with this stuff as I go through and these are my thoughts :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem - that's why it's a PR - we can discuss it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 to what @tpluscode said There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one was removed |
||
{ | ||
supportedClasses: Array<IClass>; | ||
|
||
supportedOperations: Array<IOperation>; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import {IMetadataDescription} from "./IMetadataDescription"; | ||
import "whatwg-fetch"; | ||
import {IData} from "./IData"; | ||
|
||
/** | ||
* @interface Describes an abstract meta-data providing facility which translates from raw @link Response to an abstract data model. | ||
*/ | ||
export interface IMetadataProvider | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please elaborate a bit on why this class is needed? What is the abstract data model? IData and IMetadata? Something else? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted to introduce an intermediate layer that would isolate the client and it's hypermedia controls internal data model from actual RDF serialization received. This would be a job for a IMetadataProvider to make that translation. In case of JSON-LD it would be i.e. framing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with the general idea of separation. Although I'm anxious to see how framing alone will be enough. I've been there too and I had to resort to actual triple processing as an intermediate processing step There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well - framing alone won't do it. There is already a very simple attempt to that topic in |
||
{ | ||
/** | ||
* Gets supported media types. | ||
* @returns {Array<string>} | ||
*/ | ||
supportedMediaTypes: Array<string>; | ||
|
||
/** | ||
* Parses a given raw response. | ||
* @param {Response} response Raw fetch response holding data to be parsed. | ||
* @returns {Promise<IData>} | ||
*/ | ||
parse(response: Response): Promise<IData>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it really parse? How about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Turtle implementation would actually parse it to get a JavaScript compatible objects. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, but I think that parsing is just part of what There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So maybe process - provide is already in the name of the interface thus it is also part of what it doas. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed with |
||
|
||
/** | ||
* Parses a given raw response. | ||
* @param {Response} response Raw fetch response holding data to be parsed. | ||
* @param {boolean} removeFromPayload Instructs whether to remove the metadata from the response's body or not. | ||
* @returns {Promise<IData>} | ||
*/ | ||
parse(response: Response, removeFromPayload: boolean): Promise<IData>; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import {IResource} from "./IResource"; | ||
|
||
export interface IOperation extends IResource | ||
{ | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
export interface IResource | ||
{ | ||
"@id": string; | ||
"@type"?: Array<string> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's interesting. I've nothing against There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes - I don't like @id either. The issue is that it's not something I can do with i.e. JSON-LD framing. Initially I had here an iri property. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, JSON-LD can do that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we actually use properties here and not methods? The programming interface doesn't need to match what's send on the wire There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Being just pragmatic - I'd prefer to have resource's identifier as a property. Also, we'd need to extend every single JSON object from the payload with that method's implementation - it's a drag. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something you'd still want to do to make a property read-only. A getter method would be just the same There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think this is actually a deeper question. Do we want to use JSON-LD as data representation? Both internally and accessible through Heracles' interfaces? If not, the point above is moot. If yes, this should be made explicit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No - currently I'm using JSON-LD framing to transform hypermedia controls I discover to an internal model. It is rooted in the hydra's structure, but still it is supposed to be separated from the JSON-LD serialization. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Properties renamed to |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
import HydraClient from "../../HydraClient"; | ||
import {IMetadataProvider} from "../IMetadataProvider"; | ||
import {IData} from "../IData"; | ||
import {hydra} from "../../namespaces"; | ||
const jsonLd = require("jsonld").promises; | ||
const context = require("./context.json"); | ||
|
||
export default class JsonLdMetadataProvider implements IMetadataProvider | ||
{ | ||
private static _mediaTypes = ["application/json+ld"]; | ||
private static _id = 0; | ||
|
||
static initialize() | ||
{ | ||
HydraClient.registerMetadataProvider(new JsonLdMetadataProvider()); | ||
} | ||
|
||
public get supportedMediaTypes(): Array<string> | ||
{ | ||
return JsonLdMetadataProvider._mediaTypes; | ||
} | ||
|
||
public async parse(response: Response, removeFromPayload: boolean = false): Promise<IData> | ||
{ | ||
let payload = await response.json(); | ||
let expanded = await jsonLd.flatten(payload); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/expanded/flattened/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep - forgot to replace variable name while experimenting with JSON-LD api. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed to flattened. |
||
let metadata = JsonLdMetadataProvider.parseMetadata(expanded, new Array<any>(), removeFromPayload); | ||
let result = await jsonLd.frame(metadata, context, { embed: "@link" }); | ||
result = result["@graph"]; | ||
for (let index = result.length - 1; index >=0; index--) | ||
{ | ||
if ((Object.keys(result[index]).length == 1) && (result[index]["@id"])) | ||
{ | ||
result.splice(index, 1); | ||
} | ||
} | ||
|
||
Object.defineProperty(expanded, "metadata", { value: result, enumerable: false }); | ||
return expanded; | ||
} | ||
|
||
private static parseMetadata(payload: any, result: Array<any> & { [key: string]: any }, removeFromPayload: boolean = false): any | ||
{ | ||
if (payload instanceof Array) | ||
{ | ||
return JsonLdMetadataProvider.parseArray(payload, result, removeFromPayload); | ||
} | ||
|
||
if (payload["@graph"]) | ||
{ | ||
return JsonLdMetadataProvider.parseMetadata(payload["@graph"], result, removeFromPayload); | ||
} | ||
|
||
return JsonLdMetadataProvider.parseResource(payload, result, removeFromPayload); | ||
} | ||
|
||
private static parseArray(payload: any, result: Array<any> & { [key: string]: any }, removeFromPayload: boolean = false) | ||
{ | ||
let toBeRemoved = new Array<any>(); | ||
for (let resource of payload) | ||
{ | ||
if (resource["@type"] && resource["@type"].every(item => item.indexOf(hydra.namespace) === 0)) | ||
{ | ||
Object.defineProperty(result, resource["@id"] || "_:bnode" + (++JsonLdMetadataProvider._id), { enumerable: false, value: resource }); | ||
result.push(resource); | ||
if (removeFromPayload) | ||
{ | ||
toBeRemoved.push(resource); | ||
} | ||
} | ||
else | ||
{ | ||
JsonLdMetadataProvider.parseMetadata(resource, result, removeFromPayload); | ||
} | ||
} | ||
|
||
toBeRemoved.forEach(item => payload.splice(payload.indexOf(item), 1)); | ||
return result; | ||
} | ||
|
||
private static parseResource(resource: any, result: Array<any> & { [key: string]: any }, removeFromPayload: boolean): any | ||
{ | ||
let targetResource; | ||
if ((resource["@id"]) && (targetResource = result[resource["@id"]])) | ||
{ | ||
targetResource["@id"] = resource["@id"]; | ||
} | ||
|
||
if (!targetResource) | ||
{ | ||
targetResource = {}; | ||
Object.defineProperty(result, "_:bnode" + (++JsonLdMetadataProvider._id), { enumerable: false, value: targetResource }); | ||
result.push(targetResource); | ||
} | ||
|
||
for (let property of Object.keys(resource).filter(property => property.charAt(0) !== "@")) | ||
{ | ||
if (property.indexOf(hydra.namespace) === 0) | ||
{ | ||
targetResource[property] = resource[property]; | ||
if (removeFromPayload) | ||
{ | ||
delete resource[property]; | ||
} | ||
} | ||
} | ||
|
||
return result; | ||
} | ||
} | ||
|
||
JsonLdMetadataProvider.initialize(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
{ | ||
"@context": { | ||
"@vocab": "http://www.w3.org/ns/hydra/core#", | ||
"hydra": "http://www.w3.org/ns/hydra/core#", | ||
"operations": "hydra:operation", | ||
"supportedClasses": { | ||
"@id": "hydra:supportedClass", | ||
"@container": "@set" | ||
}, | ||
"entryPoint": "hydra:entrypoint", | ||
"supportedOperations": { | ||
"@id": "hydra:supportedOperation", | ||
"@container": "@set" | ||
}, | ||
"supportedProperties": { | ||
"@id": "hydra:supportedProperty", | ||
"@container": "@set" | ||
}, | ||
"members": { | ||
"@id": "hydra:member", | ||
"@container": "@set" | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to Hydra, I'd swap these scripts. Travis will run
npm test
and it makes more sense that it was the single run scriptThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no opinion here. Whichever is better from a developer point of view (not a tool point of view)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since
start
andtest
are standard script commands, I'd say just use them. It's also possible to chain script commands together, so I'd do the following:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@asbjornu +1 for chaining.
I would suggest keeping
test
andtest-once
as names since they are action specific and related by name.Not sure if this is relevant but I remember reading that the
watch
parameter also need to be specified as false for the single run command.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to 'test' (single run) and 'live-test' (for continuous running test session)