Closed Waterstraal closed 2 years ago
Hi @Waterstraal thanks for looking into this! I'll try to run that version against our internal codebase tomorrow to make sure it doesn't break any form :)
Hi @Waterstraal thanks for looking into this! I'll try to run that version against our internal codebase tomorrow to make sure it doesn't break any form :)
Thanks, it's still WIP; it's not working 100% correct yet 😅. I'll give you a heads up when I've managed to fix it.
Ps: on my local machine 9 unit test specs fail; even on master. Is that my local setup with windows, or do you have the same? I saw all green tests on gh actions, so not sure what's going on there.
Thanks, it's still WIP; it's not working 100% correct yet sweat_smile
I've edited the PR title with a WIP prefix, please remove it from the title once it's ready :)
Ps: on my local machine 9 unit test specs fail; even on master. Is that my local setup with windows, or do you have the same?
I do have the same indeed. I double checked the pipelines and all the tests are passing though not sure whats going on there either. As it's the case on master don't hold that into account for your branch, we'll see if CI is happy with it but this should just be fixed upstream on master not in your branch :)
You can try to run the e2e tests at least, they've got a decent coverage too
@maxime1992 can you review this PR? Thanks!
Code LGTM. I'll go through some testing but it may take up to a week as we're very busy atm please bear with me. Thanks again for the PR
Small update, I've published yesterday a release for that branch and got our internal CI pipeline running based on that ngx-sub-form version. Good news is, all the e2e tests are passing on all of our apps :tada:
There's just a tiny issue with 1 e2e test but I don't have the time to investigate atm but until I figure out why that unit test is now broken I'll need to put the merge of that MR on hold. I'll try to have a look next week
@Waterstraal I've figured out why the unit I mentioned above was failing and it's all good, so I'm merging this.
Thanks for taking the time to look into the upgrade!
Fixes #272