Reuse bot object in API and optionally use chronology protector #41
Conversation
|
Oh, apparently that is a repository where we still do have some Travis-CI integration? o.O |
lucaswerkmeister
left a comment
lucaswerkmeister
left a comment
There was a problem hiding this comment.
TODO: Somehow try this out in my local browser tests
I tried it locally with npm-link, will make a patch for Gerrit/Jenkins with this approach! Edit: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikibaseLexeme/+/738378 though I may have to chain this behind https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikibaseLexeme/+/709747 to make all the peer dependencies line up correctly |
wikibase.api.js
Outdated
| }, reject ); | ||
| } ); | ||
| } ); | ||
| return new Promise( ( resolve, reject ) => { // FIXME: I don't think this Promise is needed |
There was a problem hiding this comment.
I don’t think the Promise is needed either, we can probably just await the response and then return response.entity.id? (And I’d say the same for createItem() – once we start using async/await, it seems unnecessary to keep working with .then() “manually”.)
There was a problem hiding this comment.
Yes. Though I feel like I want to split up the change to how we use mwbot and general Promise=>async/await cleanup into two commits
wikibase.api.js
Outdated
| username: browser.config.mwUser, | ||
| password: browser.config.mwPwd | ||
| } ); | ||
| } ).then( () => { |
There was a problem hiding this comment.
This line can probably be removed as well?
This should be a pure refactoring without any behavior changes
This is not a major version bump, because everything still works without using `WikibaseApi.initialize()`, it just logs a warning.
lucaswerkmeister
left a comment
lucaswerkmeister
left a comment
There was a problem hiding this comment.
Wow, even Node v8 had async/await already, we don’t even need to bump the engine version in package.json.
LGTM; do we need more testing (CI somewhere), or is this good to release?
I think this is good to go, but to be sure I think I would like the Wikibase patch to be ready similar to the Lexeme patch before merging it. We should make a ticket for replacing Travis in this repo here |
|
Ah, okay. So we’re waiting for CI to finish on https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/738421, I assume. (And, yes!) |
|
Both the Wikibase and WikibaseLexeme patches succeeded, so this is ready to be merged and a new release made |
Depends-on: wmde/wdio-wikibase#41 Bug: T277862 Change-Id: Ic46a31931a88759c041a62849a860afa5d24932c
* Update WikibaseLexeme from branch 'master'
to 7c6a2d03c5afe9e2e9441bdee2dfe7d1c13da608
- Merge "Selenium: initialize WikibaseApi with ChronologyProtector"
- Selenium: initialize WikibaseApi with ChronologyProtector
Depends-on: wmde/wdio-wikibase#41
Bug: T277862
Change-Id: Ic46a31931a88759c041a62849a860afa5d24932c
Using the chronology protector should prevent some browser test failures when running against beta and the replica hasn't caught up yet.
TODO: Somehow try this out in my local browser tests
Bug: T277862
Bug: T284443