Skip to content

Fix copying of static files - #61

Merged
bajtos merged 2 commits into
3.0from
fix/templated-files
Jul 2, 2014
Merged

Fix copying of static files#61
bajtos merged 2 commits into
3.0from
fix/templated-files

Conversation

@ritch

@ritch ritch commented Jun 30, 2014

Copy link
Copy Markdown
Member

/to @bajtos

This patch fixes file copying for addComponent().

@ritch ritch mentioned this pull request Jun 30, 2014
12 tasks
@bajtos

bajtos commented Jul 1, 2014

Copy link
Copy Markdown
Member

I am confused, the patch seems to contain two changes:

  1. fix construction of tasks, so that template files are copied
  2. add exec function that is used only in tests so far

What do you intend to use exec for?

Comment thread test/workspace.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.

Seems the exec function is not that useful at the end. What modules are missing? Perhaps they can be added to workspace dev-dependencies?

@bajtos

bajtos commented Jul 1, 2014

Copy link
Copy Markdown
Member

I am proposing the following steps:

  • Remove exec. Modify unit-tests to check for the presence of files only.
  • In a new pull request, write end-to-end test that will
    1. Create a new app from template
    2. Create node_modules folder in the project dir
    3. Run npm install in the project dir
    4. Start the scaffolded app, run the following scenario using supertest: create user, login, get user details.

That should catch most of the problems in templates. I have already fixed few of them in #63.

@ritch

ritch commented Jul 1, 2014

Copy link
Copy Markdown
Member Author

Remove exec. Modify unit-tests to check for the presence of files only.

Done. Can we merge this now?

@ritch ritch changed the title Add exec support to components Fix copying of static files Jul 1, 2014
Comment thread test/workspace.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.

This test does not assert anything, it should be removed IMO.

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.

Removed in #63.

bajtos added a commit that referenced this pull request Jul 2, 2014
@bajtos
bajtos merged commit 8acc16a into 3.0 Jul 2, 2014
@bajtos

bajtos commented Jul 2, 2014

Copy link
Copy Markdown
Member

I have merged this pull request as it was blocking my progress. The comment above can be addressed in a new pull request.

@rmg
rmg deleted the fix/templated-files branch June 21, 2016 19: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