Skip to content
This repository was archived by the owner on Oct 2, 2019. It is now read-only.

fix minimum-input-length with refresh-delay #1898 #1905

Merged
merged 2 commits into from Apr 4, 2017
Merged

fix minimum-input-length with refresh-delay #1898 #1905

merged 2 commits into from Apr 4, 2017

Conversation

ghost
Copy link

@ghost ghost commented Feb 6, 2017

@Jefiozie
Copy link
Contributor

Jefiozie commented Feb 6, 2017

Hi @danivarga , Thanks for the PR. Could you have a look again at it?

  • Think the NPM debug log can be removed

  • Can you build some tests around it?

  • think your editor has done something with indention could you fix it?

Thanks

@ghost
Copy link
Author

ghost commented Feb 7, 2017

Hi @Jefiozie,

  • Think the NPM debug log can be removed

The npm-debug.log is inside your repo. I have removed it from mine. Should i add a .gitignore rule?

  • think your editor has done something with indention could you fix it?

Indention looks fine to me. My editor just removed some trailing spaces: Line 42, Line 53, Line 56 and Line 50. This behavior is defined in your .editorconfig
Nvmd. I fixed it.

  • Can you build some tests around it?

I'll try Done. Test fails with version 0.19.5 as expected.

Thank you

@ghost
Copy link
Author

ghost commented Feb 7, 2017

I dont know why the Tests are failing in Travis.
On my local machine everything is working fine.

INFO [karma]: Karma v0.12.37 server started at http://localhost:9876/
INFO [launcher]: Starting browser Firefox
INFO [launcher]: Starting browser Chrome
INFO [Chrome 56.0.2924 (Windows 10 0.0.0)]: Connected on socket vS7cylZ9ZljV6Fgiabub with id 88188217
Chrome 56.0.2924 (Windows 10 0.0.0): Executed 208 of 208 SUCCESS (5.517 secs / 5.499 secs)
Firefox 51.0.0 (Windows 10 0.0.0): Executed 208 of 208 SUCCESS (10.002 secs / 9.961 secs)
TOTAL: 416 SUCCESS

Can you have a look at it?

Tanks

@Jefiozie
Copy link
Contributor

Jefiozie commented Feb 8, 2017

I hope I find the time later this week.

@ghost
Copy link
Author

ghost commented Feb 9, 2017

Thank you.
It looks like something is wrong with Travis. Test 59 - 62 are failing in every build even in other PRs
PR #1906 (Travis) and PR #1908 (Travis)

@Jefiozie
Copy link
Contributor

So i did some local test. Don't see why travis throws these errors. But i'm getting errors local. Could you double check on your side if there are no errors? Thanks.

@ghost
Copy link
Author

ghost commented Feb 13, 2017

Can't reproduce.
I've added Firefox and Chrome to my local karma.conf.js. Tests are passing.
image

@Jefiozie
Copy link
Contributor

Can you share your node and npm version? Thanks.

@Jefiozie
Copy link
Contributor

Hi @danivarga I think i have found the reason for the failing unit tests. Waiting for some comments why things have changed after this we can probably rebase your PR and the unit test problem will be resolved.

Thanks.

@Jefiozie
Copy link
Contributor

HI @danivarga , I have merged my PR could you rebase to the master?
Thanks.

@ghost
Copy link
Author

ghost commented Feb 23, 2017

Done :)

@Jefiozie
Copy link
Contributor

Thanks, could you maybe adjust your tests? See example I would like to get rid of all the compileTemplate and only use the function's as in the example.

@Jefiozie Jefiozie assigned ghost Mar 7, 2017
@Jefiozie
Copy link
Contributor

Any updates on this issue?

Thanks

@Jefiozie Jefiozie added this to the v0.19.6 milestone Mar 13, 2017
@Jefiozie
Copy link
Contributor

Any updates on this issue? @danivarga would like to get this merged 👍

Thanks

@Jefiozie Jefiozie merged commit 3f097e0 into angular-ui:master Apr 4, 2017
@Jefiozie
Copy link
Contributor

Jefiozie commented Apr 4, 2017

Merged but at some point we need to remove the compileTemplate and change it to createUiSelect or createUiSelectMultiple

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.

1 participant