feat(cli): `lb4 model` to scaffold model files #1487

virkt25 · 2018-06-28T15:23:35Z

implements #1221

Associated documentation changes will be in a follow up PR once this lands.

Commit Summary:

First commit fixes CLI so the base ArtifactGenerator supports updating multiple index.ts files
Second commit implemented lb4 model command
Third commit updates example-todo to match output from lb4 model command

Checklist

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

shimks

Looks 🤩.
I think in order to test the property section correctly, it's necessary to have a property generator.
For the sake of avoiding scope creep, we should do the bare minimum to make the property generator work solely with the lb4 model command.
I do remember you talking about there being a lot of work to make a feature complete property generator. Could you consider which features can be left out to make just the model command functional?

shimks · 2018-06-28T15:44:55Z

packages/cli/lib/artifact-generator.js

-          chalk.green('   update'),
-          `${this.artifactInfo.relPath}/index.ts`,
-        );
+        for (let i = 0; i < this.artifactInfo.updateIndexes.length; i++) {


any reason to not use for ... of loop here?

shimks · 2018-06-28T18:38:49Z

packages/cli/generators/model/index.js

+        type: 'list',
+        default: baseOptions[0],
+        choices: baseOptions,
+        when: baseOptions.length > 1,


Will this run? baseOptions is only populated with one element

It's not intended to run ATM but designed to run if / when we add more baseOptions in the future.

shimks · 2018-06-28T18:47:57Z

packages/cli/generators/model/index.js

+          when: function(answers) {
+            return (
+              ![null, 'buffer', 'any'].includes(answers.type) &&
+              this.typeChoices.includes(answers.type)


Is the second condition here necessary?

Oops, A remnant from the lb3 CLI which allows for custom types to be entered ... something we'll probably add later on.

shimks · 2018-06-28T19:15:33Z

packages/cli/generators/model/index.js

+
+    // Set up types for Templating
+    const TS_TYPES = ['string', 'number', 'object', 'boolean', 'any'];
+    const NON_TS_TYPES = ['geopoint', 'date'];


I thought date type would be where we'd use JavaScript's own Date like Buffer.
I think we should have something like const COMPLEX_TS_TYPES = ['buffer', 'date'] and then have a matching series of if statements to go with it

shimks · 2018-06-28T19:17:58Z

packages/cli/generators/model/index.js

+    const TS_TYPES = ['string', 'number', 'object', 'boolean', 'any'];
+    const NON_TS_TYPES = ['geopoint', 'date'];
+    Object.entries(this.artifactInfo.properties).forEach(([key, val]) => {
+      val.tsType = val.type;


Can we have this statement way down at the bottom?

Any reason to move it? Being at the top it acts as the default value without requiring a check to see if val.tsType exists or not.

I was also a bit confused when looking at this, but that makes sense 👍

shimks · 2018-06-28T19:21:57Z

packages/cli/lib/utils.js

@@ -23,6 +23,8 @@ const Conflicter = require('yeoman-generator/lib/util/conflicter');

 const readdirAsync = promisify(fs.readdir);

+const RESERVED_PROPERTY_NAMES = ['constructor'];


Let's also include access level keywords (like public) and any other applicable keywords from this list

public is actually allowed as a property on a class ... surprising but TS didn't complain when I tried it.

shimks · 2018-06-28T19:26:32Z

packages/cli/lib/utils.js

+    unallowedCharacters = /[\/@\s\+%:\.]/;
+  }
+  if (name.match(unallowedCharacters)) {
+    return `Name cannot contain special characters ${unallowedCharacters} ${name}`;


missing a colon between the two variables

Also, consider using parts of this: d6f1a64#diff-690866a08191a550353b1a1929fc6080R38

jannyHou

Great feature!! LGTM in general, left a few comments.

jannyHou · 2018-06-28T19:46:44Z

examples/todo/src/models/todo.model.json

+    "id": {
+      "type": "number",
+      "id": true,
+      "required": false


nitpick feel free to ignore: I think we can omit the required field when it's false.

jannyHou · 2018-06-28T19:48:45Z

examples/todo/src/models/_generated/todo.base.model.ts

+const modelDef = require('../todo.model.json');
+
+export class TodoBase extends Entity {
+  static definition = new ModelDefinition(modelDef);


Do we need to keep getId() function? see the old todo.model.ts file

We actually provide a getId() function in the Entity class that can check for a property with id: true and returns that. I'm actually not sure why we had an override. See: https://github.com/strongloop/loopback-next/blob/master/packages/repository/src/model.ts#L193-L212

cc: @bajtos Any ideas as to why this is overridden? Maybe I'm missing something?

jannyHou · 2018-06-28T19:55:52Z

packages/cli/generators/model/index.js

+        type: 'list',
+        default: baseOptions[0],
+        choices: baseOptions,
+        when: baseOptions.length > 1,


hmm, if the baseOptions is hard-coded on Line71, how would this prompt be triggered?

It's not intended to run ATM but designed to run if / when we add more baseOptions in the future.

I usually prefer to leave out code that will be used only later in the future because of YAGNI, but don't mind too much to have it here.

Took it out -- it's simple enough logic that it can be added without pain when needed.

jannyHou · 2018-06-28T20:03:15Z

packages/cli/generators/model/index.js

+
+        Object.assign(this.artifactInfo.properties[this.propName], props);
+        if (props.id) {
+          this.idFieldSet = true;


I love the check here to prevent user from answering the question everytime when add a new property.
But not sure would it make sense for composed id, see https://loopback.io/doc/en/lb3/Model-definition-JSON-file.html#id-properties
IMO composed id is out of the scope of this PR though.

I'll create a follow up issue to discuss composite IDs and how we will support them.

bajtos

Before we focus on implementation details, let's discuss the changes at high-level please. I'll post a longer comment in a moment.

bajtos · 2018-06-29T07:43:38Z

This pull request changes the way how models are defined by app developers. So far, we were using TypeScript decorators only. The proposal here is to switch to JSON-based configuration. I am personally not happy with that direction, as it forces developers to maintain model definitions in two places (TypeScript source containing TS API, a JSON file containing LB definition).

I read through #1221 and did not find any mention of this change. In the opposite, the issue explicitly says:

The generated model.ts would look something like: https://github.com/strongloop/loopback-next/blob/0411811aa8348e31636fb0fe07b44dd18469280f/examples/todo/src/models/todo.model.ts
At the end, src/models/customer.model.ts will be created.

I would like the generator to emit files and source code that's following the current approach as shown in examples/todo and probably other places too:

All model settings and property definitions stay in a single place - model's TypeScript source file located in src/models/{name}.ts
There is only a single class created for a model (e.g. Todo), no base class (e.g. TodoBase)

Thoughts?

virkt25 · 2018-06-29T20:23:09Z

Game Plan

Talked to both @bajtos and @raymondfeng to discuss the concerns and what we want. Here's the plan to land this feature (incremental, iterative approach).

This PR will only introduce code generation to generate something similar to the current todo.model.ts -- no JSON / Base Class.
Make tests more robust as discussed with Raymond to include coverage for property generation (using an index to make each property name unique).
Use new syntax instead of .bind in certain places it's being used.
Address other feedback from reviews by @jannyHou and @shimks (Thanks!)

Out of Scope of this PR:

Declarative Support, I'll open a new issue with more details on the requirements for discussion and schedule any meetings if we can't have consensus online. I'll post a link here once the issue is created.

virkt25 · 2018-06-29T21:24:11Z

UPDATE: Issue for discussing declarative support for models: #1491

virkt25 · 2018-07-07T03:52:32Z

The two .js.bak files ... particularly the generator and the test file is to show that using a unique name for each property did not make the property generation aspect of this generator testable as it gets stuck despite using unique names as the functions are recursively called and prompts may be passed in all at once.

The files will be removed before merging.

Basic tests are present
Index Generation to allow update to multiple index.ts is included (I added for _generated index.ts but it's removed and no longer used but leaving it in for future -- if needed) -- if there are objections I can remove it
No more _generated / json files. Just a simple .model.ts file. There is a formatting issue. TODO (Out of Scope of this PR): Add in support to have prettier format the files based on a project config. -- Create issue.
Update Docs -- Follow up PR once this lands

shimks

Have you considered restructuring the way property prompts are run to accommodate for the tests? Since there's no way to recursively call the property prompt, maybe we can have our code internally support this use case:

const prompt = {
  name: 'test',
  properties: {
    prop1: {propName: 'foo'},
    prop2: {propName: 'bar'},
  }
}
await testUtils.prompt(prompt)

And have the code look something like this:

async promptPropertyPrompts {
  // somehow prompt for a false prompt that only sets up the `properties` key
  // and at the same time prevent the logger from showing this process to the user

  // this prompt would somehow populate the `properties` key set up earlier
  this._promptProperty();
}

Admittedly, this is a very hack way of doing things. An alternative that we could consider is to do what we did here https://github.com/strongloop/loopback-next/blob/master/packages/cli/test/integration/lib/artifact-generator.js#L108

It might just be easier to just make a property generator 😂

EDIT: some review comments are hidden since I reviewed through each commits. Aside from the insignificant comment on async/await stuff, they're still valid though

shimks · 2018-07-09T11:42:29Z

examples/todo/src/models/todo.model.ts

@@ -27,22 +27,18 @@ export class Todo extends Entity {
  @property({
    type: 'boolean',
  })
-  isComplete: boolean;
+  isComplete?: boolean;


Any reason to change this to optional? If we do change it, could you also find references to this model in the documentation and fix that up as well?

FWIW, the decoration @property({type: 'boolean'}) means that the property is optional at least as far as the persistence layer is concerned (juggler & connectors).

Only reason is this is how the CLI generated file will look ... I wanted to keep it consistent for users looking at this vs. what they get when they follow the tutorial.

Docs (including tutorial) will be updated in a follow up PR.

shimks · 2018-07-09T12:00:34Z

packages/cli/generators/model/index.js

-    return this.prompt(prompts).then(props => {
-      if (!props.base) props.base = baseOptions[0];
-      Object.assign(this.artifactInfo, props);
+    const answers = await this.prompt(prompts);


Never knew this would work; I thought this.prompt() never actually returned a promise. TIL'ed

shimks · 2018-07-09T12:09:21Z

packages/cli/generators/model/index.js

+      val.type = `'${val.type}'`;
+
+      if (!val.required) {
+        delete val.required;


Is this necessary? What I'm getting from this is if val.required does not exist, delete the key off of val. Is the code manually defining the key as undefined or null somewhere?

The code is actually designed to delete val.required if required is set to false based on feedback from @jannyHou ... we don't need to decorate the property with required: false since that's the default if not present.

shimks · 2018-07-09T12:12:42Z

packages/cli/lib/artifact-generator.js

-          await updateIndex(update.dir, update.file);
+      if (!this.artifactInfo.disableIndexUpdate) {
+        if (
+          !this.artifactInfo.updateIndexes &&


We should change the name updateIndexes to something like indexesToBeUpdated since it could be functionally confusing with disableIndexUpdate

bajtos · 2018-07-09T15:22:21Z

examples/todo/src/models/todo.model.ts

@@ -27,22 +27,18 @@ export class Todo extends Entity {
  @property({
    type: 'boolean',
  })
-  isComplete: boolean;
+  isComplete?: boolean;


FWIW, the decoration @property({type: 'boolean'}) means that the property is optional at least as far as the persistence layer is concerned (juggler & connectors).

bajtos · 2018-07-09T15:22:41Z

examples/todo/src/models/todo.model.ts


  // TODO(bajtos) Use LoopBack's GeoPoint type here
  @property({
    type: 'string',
  })
-  remindAtGeo: string; // latitude,longitude
-
-  getId() {


What happened to this function - are you sure we don't need it any more?

Should we perhaps modify the template used by the CLI tool to always include getId() implementation, preferably after the constructor function?

I think with the refactor my comment on GitHub got lost somewhere ... but the reason for me to not include the getId function is as follows:

The underlying Entity class actually implements it based on the metadata ... so any property can be the id field and it handles it correctly (and while we can do this with the CLI ... I don't see the point of overriding the logic from the base class.

Here's the reference code: https://github.com/strongloop/loopback-next/blob/08c2d896367a7f8ecc5153578f2a698eefdea7a1/packages/repository/src/model.ts#L218-L231

Good point. I guess my concern is about performance - an explicit getId implementation returning this.id is surely faster than the generic solution processing model definition.

But as Donald Knuth said, premature optimization is the root of all evil. I am ok with your proposed change, we can always add explicit getId() implementation later, when there is a legitimate need.

bajtos · 2018-07-09T15:25:20Z

packages/cli/generators/model/index.js

+        type: 'list',
+        default: baseOptions[0],
+        choices: baseOptions,
+        when: baseOptions.length > 1,


I usually prefer to leave out code that will be used only later in the future because of YAGNI, but don't mind too much to have it here.

The comments have been already addressed.

bajtos · 2018-07-09T15:33:38Z

using a unique name for each property did not make the property generation aspect of this generator testable as it gets stuck despite using unique names as the functions are recursively called and prompts may be passed in all at once.

I think this is sort of a pain point known for long time, see [CLI] Get rid of Yeoman #844:

AFAICT, there is no way how to test non-trivial prompting logic - for example the loop asking for multiple properties. A mocked prompt can define only a single set of answers. Running the generator/CLI end-to-end and parsing stdout is brittle because Inquirer uses lots of ANSI control codes to control what's displayed on the screen.

bajtos

LGTM 👍

Please don't forget to remove the two .bak files before landing.

bajtos · 2018-07-10T12:13:17Z

packages/cli/lib/artifact-generator.js

-          chalk.green('   update'),
-          `${this.artifactInfo.relPath}/index.ts`,
-        );
+      if (!this.artifactInfo.disableIndexUpdate) {


Could you please use reverse-logic and early return to reduce the amount of indentation and make the code easier to reason about?

if (this.artifactInfo.disableIndexUpdate) return;

I think we should move the code handling index updates into a new method, the end method is already too long.

class ArtifactGenerator extends BaseGenerator { // ... async end() { // ... if (generationStatus) { this._updateIndexFiles(); } // User Output this.log(); // ... } async _updateIndexFiles() { if (this.artifactInfo.disableIndexUpdate) return; // ... } }

Thoughts?

On top of this, I have a question rather than a suggestion. Is this new logic covered by the following test in model.integration.js?

describe('generator-loopback4:model', tests);

bajtos · 2018-07-10T12:18:32Z

packages/cli/lib/artifact-generator.js

+
+          this.log(
+            chalk.green('   update'),
+            `${this.artifactInfo.relPath}/${updateDirRelPath}${


I find multi-line templates difficult to read. Have you considered extracting individual bits into local variables?

const relPath = this.artifactInfo.relPath; const sep = updateDirRelPath.length > 0 ? '/' : ''; const indexPath = `${relPath}/${updateDirRelPath}${sep}index.ts`; this.log(chalk.green(' update'), indexPath);

I think it would be even better to use Node.js core API path.join(). Possible caveat: it may use \ instead of / on Windows (don't remember what's the exact behavior), IDK if that would be a good or a bad thing.

const indexPath = path.join( this.artifactInfo.relPath, updateDirRelPath, 'index.ts' ); this.log(chalk.green(' update'), indexPath);

The function handles empty path segments well:

$ node > path.join('/asd', '', 'index.ts') '/asd/index.ts'

b-admike · 2018-07-10T17:04:33Z

packages/cli/test/integration/generators/model.integration.js

+const expect = testlab.expect;
+const TestSandbox = testlab.TestSandbox;
+
+// const ModelGenerator = require('../../../generators/model');


Can we remove this commented line? (IMO we might want to rename the variable generator below to modelGenerator too while you're at it)

b-admike · 2018-07-10T18:08:34Z

packages/cli/lib/artifact-generator.js

-          chalk.green('   update'),
-          `${this.artifactInfo.relPath}/index.ts`,
-        );
+      if (!this.artifactInfo.disableIndexUpdate) {


On top of this, I have a question rather than a suggestion. Is this new logic covered by the following test in model.integration.js?

describe('generator-loopback4:model', tests);

b-admike · 2018-07-10T18:14:36Z

packages/cli/generators/model/index.js

+const utils = require('../../lib/utils');
+const chalk = require('chalk');
+const path = require('path');
+const _ = require('lodash');


I think lodash is an unused import

Good catch!

b-admike · 2018-07-10T18:18:35Z

packages/cli/generators/model/index.js

+'use strict';
+
+const ArtifactGenerator = require('../../lib/artifact-generator');
+const debug = require('../../lib/debug')('model-generator');


ditto with debug, it's not used in this file; consider using it to log the answers we get back from the prompts perhaps or removing it altogether (none of the other generators have it). Also, same applies to the datasource generator index.js file.

Good catch. Added debug for property names and property info. debug is used in the datasource generator ...

oops :-), never mind in that case.

b-admike · 2018-07-10T18:22:00Z

packages/cli/generators/model/index.js

+    const TS_TYPES = ['string', 'number', 'object', 'boolean', 'any'];
+    const NON_TS_TYPES = ['geopoint', 'date'];
+    Object.entries(this.artifactInfo.properties).forEach(([key, val]) => {
+      val.tsType = val.type;


I was also a bit confused when looking at this, but that makes sense 👍

b-admike

Most of my comments are nitpicks, consider applying them before landing (and removing the two .bak files), but other than that LGTM.

virkt25 · 2018-07-10T23:14:51Z

This comment got collapsed:

On top of this, I have a question rather than a suggestion. Is this new logic covered by the following test in model.integration.js?

No, the new logic for index.ts updates is not included in model.integration.js simply because the logic lives in artifact.generator.js -- since it's shared by all artifacts. We never had a test for the original logic (Which I wrote ... there is a unit test for the actual updateIndex function this logic calls however). For the additional logic within the artifact generator I'm actually not sure how to test it in isolation as this requires certain properties to be present on artifactInfo such as relPath, outDir, outFile, etc. -- which means the entire generator would have to be run :/

If you have any suggestions for this please let me know and I'll add the tests in a follow up PR.

raymondfeng · 2018-07-11T14:38:30Z

packages/cli/lib/artifact-generator.js

-          `${this.artifactInfo.relPath}/index.ts`,
-        );
-      }
+      await this._updateIndexFiles();


Should updateIndexFiles be a step before end?

It's part of end at the moment.

See http://yeoman.io/authoring/running-context.html

Moving it would mean also moving/duplication the generationStatus check.

raymondfeng · 2018-07-11T14:56:43Z

packages/cli/lib/artifact-generator.js

@@ -93,27 +93,7 @@ module.exports = class ArtifactGenerator extends BaseGenerator {
    });

    if (generationStatus) {


Ideally, I think we should generationStatus file by file now instead of a global flag. For example,

models/model-1.ts (success)
models/model-2.ts (skipped)

Would it be ok if I made a follow up task for this -- I assume this is needed for lb4 openapi which can call multiple generators under the hood?

b-admike · 2018-07-11T18:25:17Z

No, the new logic for index.ts updates is not included in model.integration.js simply because the logic lives in artifact.generator.js -- since it's shared by all artifacts. We never had a test for the original logic (Which I wrote ... there is a unit test for the actual updateIndex function this logic calls however). For the additional logic within the artifact generator I'm actually not sure how to test it in isolation as this requires certain properties to be present on artifactInfo such as relPath, outDir, outFile, etc. -- which means the entire generator would have to be run :/

If you have any suggestions for this please let me know and I'll add the tests in a follow up PR.

I don't know enough of this package to suggest approaches but from what you've described, it makes sense why we didn't include tests for it.

virkt25 requested review from bajtos and raymondfeng as code owners June 28, 2018 15:23

virkt25 force-pushed the lb4-model branch from 2b6e785 to 3fa9b28 Compare June 28, 2018 15:26

virkt25 requested a review from a team June 28, 2018 15:27

virkt25 changed the title ~~Lb4 model~~ feat(cli): lb4 model to scaffold model files Jun 28, 2018

shimks reviewed Jun 28, 2018

View reviewed changes

jannyHou reviewed Jun 28, 2018

View reviewed changes

bajtos previously requested changes Jun 29, 2018

View reviewed changes

virkt25 force-pushed the lb4-model branch from c40ca5b to 08bb623 Compare July 6, 2018 02:42

virkt25 changed the title ~~feat(cli): lb4 model to scaffold model files~~ [WIP - DO NOT REVIEW] feat(cli): lb4 model to scaffold model files Jul 6, 2018

virkt25 changed the title ~~[WIP - DO NOT REVIEW] feat(cli): lb4 model to scaffold model files~~ feat(cli): lb4 model to scaffold model files Jul 7, 2018

shimks reviewed Jul 9, 2018

View reviewed changes

bajtos reviewed Jul 9, 2018

View reviewed changes

virkt25 force-pushed the lb4-model branch from 47cdc7f to 2e48ae5 Compare July 10, 2018 03:41

bajtos approved these changes Jul 10, 2018

View reviewed changes

b-admike reviewed Jul 10, 2018

View reviewed changes

b-admike approved these changes Jul 10, 2018

View reviewed changes

virkt25 force-pushed the lb4-model branch from 4b19558 to a61fa52 Compare July 10, 2018 18:38

virkt25 force-pushed the lb4-model branch from 6739e23 to da0c881 Compare July 10, 2018 23:39

shimks approved these changes Jul 11, 2018

View reviewed changes

raymondfeng reviewed Jul 11, 2018

View reviewed changes

virkt25 force-pushed the lb4-model branch from da0c881 to 625775d Compare July 12, 2018 12:44

virkt25 added 3 commits July 12, 2018 19:47

fix(cli): support updating multiple index.ts files

1b8ec51

feat(cli): lb4 model command to scaffold model files

01c687b

refactor(example-todo): update files to match output from lb4 model

854a1c6

virkt25 force-pushed the lb4-model branch from 625775d to 854a1c6 Compare July 12, 2018 23:47

virkt25 merged commit 9d5af7e into master Jul 13, 2018

virkt25 deleted the lb4-model branch July 13, 2018 00:14

virkt25 mentioned this pull request Jul 13, 2018

[CLI] Update generationStatus handling in ArtifactGenerator #1524

Closed

3 tasks

		@@ -23,6 +23,8 @@ const Conflicter = require('yeoman-generator/lib/util/conflicter');

		const readdirAsync = promisify(fs.readdir);

		const RESERVED_PROPERTY_NAMES = ['constructor'];

		@@ -93,27 +93,7 @@ module.exports = class ArtifactGenerator extends BaseGenerator {
		});

		if (generationStatus) {

feat(cli): lb4 model to scaffold model files #1487

feat(cli): lb4 model to scaffold model files #1487

Conversation

virkt25 commented Jun 28, 2018

Checklist

shimks left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jannyHou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jannyHou Jun 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jannyHou Jun 28, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bajtos left a comment

Choose a reason for hiding this comment

bajtos commented Jun 29, 2018

virkt25 commented Jun 29, 2018

virkt25 commented Jun 29, 2018

virkt25 commented Jul 7, 2018 • edited

shimks left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bajtos commented Jul 9, 2018

bajtos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

b-admike left a comment

Choose a reason for hiding this comment

virkt25 commented Jul 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

b-admike commented Jul 11, 2018

feat(cli): `lb4 model` to scaffold model files #1487

feat(cli): `lb4 model` to scaffold model files #1487

shimks left a comment •

edited

jannyHou Jun 28, 2018 •

edited

jannyHou Jun 28, 2018 •

edited

virkt25 commented Jul 7, 2018 •

edited

shimks left a comment •

edited