Add hash to Snak #77
Add hash to Snak #77
Conversation
|
does look sane |
README.md
Outdated
| ### 3.1.0 (dev) | ||
|
|
||
| * Added `hash` parameters to the `Snak` class constructors | ||
| and a `getHash()` function to the `Snak` class. |
There was a problem hiding this comment.
I think this should mention the actual classes that got a new constructor parameter. For two of them the new parameter is the 2nd, for one it's the 3rd.
lucaswerkmeister
force-pushed
the
snakHash
branch
from
98fb646
to
3e1b06a
Compare
|
I’ve added some tests, but I couldn’t figure out how to run the tests locally, so I’ll wait for CI to tell me if they pass :) |
lucaswerkmeister
force-pushed
the
snakHash
branch
2 times, most recently
from
e4abc65
to
0969deb
Compare
npm test? |
|
|
lucaswerkmeister
force-pushed
the
snakHash
branch
2 times, most recently
from
39a87d7
to
0c6fb17
Compare
The Snak class gains a _hash field and getHash() getter function, and its constructors get a new, optional hash parameter.
lucaswerkmeister
force-pushed
the
snakHash
branch
from
0c6fb17
to
d17091d
Compare
|
Okay, |
| @@ -21,11 +21,11 @@ var testSets = [ | |||
| * @return {wikibase.datamodel.Snak} | |||
| */ | |||
| function constructSnak( SnakConstructor, params ) { | |||
| return new SnakConstructor( params[0], params[1] ); | |||
| return new SnakConstructor( params[0], params[1], params[2] ); | |||
There was a problem hiding this comment.
This is pretty awkward, because this code always passes three constructor arguments, even if there are only two for two of the subclasses. But the test setup was like this before and the change made in this patch doesn't make it worse.
| [wb.datamodel.PropertySomeValueSnak, ['P1']], | ||
| [wb.datamodel.PropertyValueSnak, ['P1', new dv.StringValue( 'test' )]], | ||
| [wb.datamodel.PropertyValueSnak, ['P2', new dv.StringValue( 'test' )]] | ||
| [wb.datamodel.PropertyNoValueSnak, ['P1', undefined]], |
There was a problem hiding this comment.
I think the two "undefined" can be omitted, but it doesn't hurt to have them.
There was a problem hiding this comment.
No, they’re necessary because the expected hash is the last array element (array[array.length-1]).
There was a problem hiding this comment.
Ah, thanks! I see it now.
The
Snakclass gains a_hashfield andgetHash()getter function, and its constructors get a new, optionalhashparameter.