Skip to content

More yeoman changes - #63

Merged
bajtos merged 8 commits into
3.0from
more-yeoman-changes
Jul 2, 2014
Merged

More yeoman changes#63
bajtos merged 8 commits into
3.0from
more-yeoman-changes

Conversation

@bajtos

@bajtos bajtos commented Jul 1, 2014

Copy link
Copy Markdown
Member
  • Omit extra properties from json files
  • Fix name in the root package.json
  • Fix serialization of component models
  • Move restApiRoot from api-server to server
  • Omit json config files in the root component
  • templates: clean up + upgrade to loopback-boot 2.0
  • Mark all componentName properties as required.
  • Implement Workspace.isValidDir

/to @ritch please review

Close #34

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'd like to modify the workspace entities, rename ComponentDefinition to Component and move the content of config.json to a new entity ComponentConfig.

That way we won't have to create a dummy ComponentDefinition when there is not config.json in the component, as is the case with the root component of api-server.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the only issue that we are generating an empty config.json?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We are not generating any config.json file.

I also don't like that config.json is embedded in ComponentDefinition, since config.json does not contain anything related to component definition. Instead, it contains arbitrary component configuration used by the component itself.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Let's continue the discussion in #66.

This was referenced Jul 1, 2014
Comment thread connector.js

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Whats up with this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The tests were complaining about a possible leak when 11 listeners were registered.

@bajtos bajtos added this to the 3.0 milestone Jul 1, 2014
Comment thread models/config-file.js Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Check the error code to ensure it is a parser error. Otherwise the error might be confusing (eg. too many fds open, permissions, ENOENT, etc).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The isn't a good error code for JSON parsing errors AFAICT.

Perhaps it is enough to change the message to 'Cannot load ' + configFile.path + ...?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added a check err.name === 'SyntaxError'.

@bajtos

bajtos commented Jul 1, 2014

Copy link
Copy Markdown
Member Author

@ritch LTGY now?

@ritch

ritch commented Jul 1, 2014

Copy link
Copy Markdown
Member

LGTM

Miroslav Bajtoš added 8 commits July 2, 2014 09:38
Fix ComponentDefinition.saveToFs to remove extra properties before
writing json files (config, models, datasources).
Fix `Workspace.createFromTemplate` to set the root package name
to the value supplied by the user, instead of hard-coded `.`.
The `models.json` should contain only models of the current component.
The root project does not have an app to load config.json.

The configuration of REST API endpoint belongs to `server` component,
because that's the component registering all different sub-apps.
The `api-server` root component should not include the following files,
since there is no `app` to read them during boot:
 - config.json
 - datasources.json
 - models.json
 - Include `loopback-boot 2.x` in the root `package.json`

 - Include express 4.x middleware packages in the root `package.json`

 - Drop `email` datasource. It is no longer needed, since loopback-boot
   does not call `loopback.autoAttach`.

 - Rename model names in `rest/models.json` to reference the built-in
   loopback models. Make internal models like `AccessToken` not public.

 - Refactor the `server` template to use the boot-based registration of
   middleware (see loopback-example-full-stack).
A basic implementation that checks whether all components can be loaded
and whether there is at least one component present.
@bajtos

bajtos commented Jul 2, 2014

Copy link
Copy Markdown
Member Author

Rebased on top of current 3.0.

bajtos added a commit that referenced this pull request Jul 2, 2014
@bajtos
bajtos merged commit 279b3ff into 3.0 Jul 2, 2014
@bajtos
bajtos deleted the more-yeoman-changes branch July 2, 2014 07:43
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.

2 participants