Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Intern 4 (again) #733

Merged
merged 7 commits into from Oct 27, 2017
Merged

Intern 4 (again) #733

merged 7 commits into from Oct 27, 2017

Conversation

bryanforbes
Copy link
Member

Type: enhancement

The following has been addressed in the PR:

  • There is a related issue
  • All code matches the style guide
  • Unit or Functional tests are included in the PR

Description:

Rather than try to figure out the merge conflicts from master and Ed's PR, I did a clean conversion and cherry picked things from Ed's branch. One note: I did not indent the tests: {} blocks to make it easier to review. After approval and before merging, I will indent the tests: {} blocks.

Resolves #617

Copy link
Member

@agubler agubler left a comment

Choose a reason for hiding this comment

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

Couple of minor comments

import * as registerSuite from 'intern!object';
import * as assert from 'intern/chai!assert';
const { registerSuite } = intern.getInterface('object');
const { assert } = intern.getPlugin('chai');
// import pollUntil = require('intern/dojo/node!leadfoot/helpers/pollUntil');
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this commented import?

}
rAFStub.reset();
}

function resolveRIC() {
for (let i = 0; i < rICStub.callCount; i++) {
rICStub.getCall(0).args[0]();
rICStub.getCall(i).args[0]();
Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

const factoryRegistry = new Registry();
factoryRegistry.define('my-widget', promise);
factoryRegistry.get('my-widget');
resolveFunction(WidgetBase);
Copy link
Member

Choose a reason for hiding this comment

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

Isn’t the point of this test to resolve after the get?

Copy link
Member Author

Choose a reason for hiding this comment

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

True. I'll fix this and the next test.

@dylans dylans added this to the 2017.10 milestone Oct 27, 2017
@bryanforbes bryanforbes merged commit b626059 into dojo:master Oct 27, 2017
@bryanforbes bryanforbes deleted the intern-4-again branch October 27, 2017 16:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants