Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update LB version to LB3 #539

Merged
merged 1 commit into from
Dec 6, 2018
Merged

Update LB version to LB3 #539

merged 1 commit into from
Dec 6, 2018

Conversation

nabdelgadir
Copy link
Contributor

Description

Update LoopBack version from LoopBack 2 to LoopBack 3.

Related issues

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide

.gitignore Outdated Show resolved Hide resolved
common/models/config-file.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
server/model-config.json Outdated Show resolved Hide resolved
server/server.js Outdated Show resolved Hide resolved
@bajtos
Copy link
Member

bajtos commented Nov 29, 2018

@nabdelgadir The code base in loopback-workspace is ancient and some files (e.g. server/server.js) have not been updated to match the changes in project layout we made since early 1.x days 😞

Anyhow, here are the lines where to start learning about workspace internals:

boot(app, __dirname);
// file persistence
require('./connector');

What is happening here: we are modifying memory connector in such way that queries are loading data from project files and updates are modifying project files. The implementation details shared by all models are in server/connector.js. Then there are entity methods Entity.allFromCache & Entity.saveToFs dealing with writing data to the filesystem, and ConfigFile.findFacetFiles, Facet.loadIntoCache etc. dealing with reading data from the filesystem.

I think the code inside individual model classes (Entity subclasses) should not need any changes to work in LoopBack 3.x. The code inside server/connector.js may need small tweaks to make it work with 3.x version of the memory connector, although it's also possible no changes will be needed.

@nabdelgadir nabdelgadir force-pushed the update-loopback branch 2 times, most recently from d4e9cdd to d3ea6dd Compare November 29, 2018 14:38
server/server.js Outdated Show resolved Hide resolved
@bajtos
Copy link
Member

bajtos commented Nov 29, 2018

ok, the changes made so far looks good to me.

Now we need to find out why the tests are failing.

 1) ComponentConfig "before each" hook: givenBasicWorkspace for "should read data from "component-config.json"":
     Error: Cannot call WorkspaceEntity.destroyAll(). The destroyAll method has not been setup. The PersistedModel has not been correctly attached to a DataSource!

@nabdelgadir nabdelgadir force-pushed the update-loopback branch 4 times, most recently from 149c851 to 4dee7b4 Compare December 3, 2018 18:11
@bajtos bajtos changed the title [WIP] update LB version to LB3 Update LB version to LB3 Dec 4, 2018
@nabdelgadir nabdelgadir force-pushed the update-loopback branch 2 times, most recently from 92106f6 to d728c2f Compare December 4, 2018 15:06
@nabdelgadir
Copy link
Contributor Author

@slnode test please

@nabdelgadir nabdelgadir force-pushed the update-loopback branch 3 times, most recently from 3010008 to 248d775 Compare December 4, 2018 20:06
Copy link
Member

@b-admike b-admike left a comment

Choose a reason for hiding this comment

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

Nice work 👍

@bajtos
Copy link
Member

bajtos commented Dec 6, 2018

I see the downstream builds for generator-loopback are still failing. While I think those test failure should not be related to this pull request, we cannot be really sure.

Let's fix generator-loopback first please. Then trigger a new build and see if all is good.

I opened a pull request to fix generator-loopback tests, see strongloop/generator-loopback#399 Once it's approved & merged, we should back-port it to 5.x branch of generator-loopback too.

cc @jannyHou

Co-authored-by: Miroslav Bajtos <[email protected]>
@bajtos
Copy link
Member

bajtos commented Dec 6, 2018

@slnode test please

@bajtos
Copy link
Member

bajtos commented Dec 6, 2018

@nabdelgadir all CI checks are green now, yay! 🎉

@nabdelgadir nabdelgadir merged commit 9b62185 into master Dec 6, 2018
@nabdelgadir nabdelgadir deleted the update-loopback branch December 6, 2018 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants