Skip to content

Conversation

@bajtos
Copy link
Member

@bajtos bajtos commented Jan 26, 2018

The first commit 929737c brings in an exact copy of https://github.com/strongloop/loopback4-example-rpc-server.

The second commit f8d08a5 contains changes necessary to integrate the example with the rest of the monorepo.

See #836

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • Related API Documentation was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in packages/example-* were updated

@bajtos bajtos added this to the Sprint 54 milestone Jan 26, 2018
@bajtos bajtos self-assigned this Jan 26, 2018
@bajtos bajtos requested review from kjdelisle and virkt25 January 26, 2018 14:13
@bajtos bajtos requested a review from raymondfeng as a code owner January 26, 2018 14:13
npm i -g @loopback/cli
```

2. Download the "getting-started" application.
Copy link
Contributor

@b-admike b-admike Jan 26, 2018

Choose a reason for hiding this comment

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

getting-started -> rpc-server

"loopback-application",
"loopback"
],
"main": "./dist/src/index.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this point to index.js?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually it appears that index.d.ts and index.js are missing in this example yet it supports Node 6 so I'm actually confused as to how this even passes on 6 rn.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should; the original project was made before the CLI tooling existed.

@@ -0,0 +1 @@
--recursive
Copy link
Contributor

Choose a reason for hiding this comment

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

This file can be removed as it's not used by the test script in package.json

@@ -0,0 +1,17 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to a tslint file when using lb-tslint. This file can be removed.

@@ -0,0 +1,4 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

This file can be removed. None of the other example-* repos have it.


3. Switch to the directory and install dependencies.
```
cd loopback4-example-getting-started && npm i
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget about these, either! :)

"loopback-application",
"loopback"
],
"main": "./dist/src/index.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

It should; the original project was made before the CLI tooling existed.

@bajtos bajtos force-pushed the example/rpc-server branch from f8d08a5 to 84dcb6d Compare January 29, 2018 08:56
@bajtos
Copy link
Member Author

bajtos commented Jan 29, 2018

Thank you @kjdelisle and @virkt25 for your detailed review. So many trivial mistakes, I feel bad about not being more diligent when checking my changes myself.

I believe I have addressed all your comments now (see 84dcb6d).

@bajtos
Copy link
Member Author

bajtos commented Jan 29, 2018

FWIW, the build is failing because of a know problem in one of our dependencies, see sinonjs/nise#27 and sinonjs/nise#29

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.

5 participants