added option to change known properties at runtime

maxlath / wikibase-edit

a lib to edit Wikibase from NodeJS

67 stars 25 forks source link

added option to change known properties at runtime #35

Closed SirkoS closed 5 years ago

SirkoS commented 5 years ago

this adds the option to set the properties used by wikibase-edit within the runtime configuration instead of having to update the dependency manually.

when working with other wikibase instances I guess this makes it easier to maintain a custom set of properties without having to fiddle around in the dependencies.

maxlath commented 5 years ago

hey @SirkoS, thanks for this PR! unfortunately, there is already a branch addressing this issue, and more generally getting rid of wikidata.org coupling and renaming the module wikibase-edit. The approach taken by this branch is to query properties datatypes only when needed, and directly from the Wikibase API: even less configuration to do \o/ could you give it a try? NB: there are several yet undocumented breaking changes, so you can also just wait for the new version to be documented and published

SirkoS commented 5 years ago

I missed that. I just saw some comments here and there about that split, but didn't spot the branch.

I'll try to test that branch with my code. Any ETA for when you'll publish that version will be available via npm?

maxlath commented 5 years ago

@SirkoS hopefully in the coming week

SirkoS commented 5 years ago

Sounds good!

I was able to rewrite my code using you branch. So from my side everything seems fine.

While looking through your code for the mentioned changes, I noted some things:

the check in wrappers_utils.js/validateAndEnrichConfig() seems wrong right now. I'm guessing you moved username/password under credentials in the config object, so that line should look like this:
```
if (config.credentials.oauth && (config.credentials.username || config.credentials.password)) {
```
you currently rely on your own (?) request wrapper bluereq. why not just use the "official" one request-promise-native? This would also remove the (indirect) bluebird dependency. As you use native promises on other occasions already, there is no need for bluebird in my eyes.
following the previous note: if you bump the supported node version a little, you could make use of asnyc/await, which would also simplify the code in my eyes.

maxlath commented 5 years ago

validateAndEnrichConfig has been fixed in 5b1b634
there is no official request wrapper, just different public relations strategies ;) that said, I agree we should move to something else
not using async/await means no transition work and better support for older NodeJS versions, so while I see the readability benefit, I would like to keep procrastinating a bit on this :)

maxlath commented 5 years ago

@SirkoS wikibase-edit v3.0.1 has been published! see changelogs

SirkoS commented 5 years ago

@SirkoS wikibase-edit v3.0.1 has been published! see changelogs

Great! I'm sorry about abusing this thread and my constant nagging, but I found two other minor issues:

in the changelog the link to "create property" uses the wrong hash right now (looks like a c&p error)
the npm banner in the README.md shows the wikibase-sdk data and not the one from wikibase-edit (the link is fine, however)

(I'll close the request here)

maxlath commented 5 years ago

@SirkoS changelog and banner fixed, thanks for reporting!