Closed ozcanxbreeze closed 1 year ago
@ozcanxbreeze I see that you pushed commit https://github.com/CrossBreezeNL/crossmodel/pull/22/commits/693e8fc7d5c177e01905c3729376e53c1414fe3d just now. I did not consider this in my review as I started before that. So please disregard any comments that are outdated because of it.
Hey Martin, Thanks for the review. I indeed did push some changes, because I wanted to switch branches to review your pull request. I did not manage to get it functionally working yet, so https://dev.azure.com/x-breeze/CrossModel/_workitems/edit/834 is still open.
I also just merged your branch into main, so I will have to do some merging and refactoring to get it working again.
@ozcanxbreeze I pushed an update that should resolve the issue we faced with the broken build. Basically what caused the problem was that in the model-service
package the index was put outside of the browser
and node
directory but it exported the files from there. So when we did an import from that index from the frontend, it automatically tried to resolve the exported node files as well and then broken with the missing fs
dependency that was introduced by my commit in the meantime.
Please note that I force pushed but I kept your changes.
@martin-fleck-at Awesome, I am going to resolve the pull request review issues, and then I will notify you again
@martin-fleck-at Hey Marin, I just pushed a new commit that implement your review points and your remarks.
A pull request for the following tasks:
check for documentation
Also did the following to get it working:
Made new tasks for the following todo's: