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

Get rid of package.json generation and yeoman generator #527

Merged
merged 5 commits into from
Sep 25, 2017

Conversation

akosyakov
Copy link
Member

No description provided.

@akosyakov akosyakov force-pushed the ak/get_rid_of_package_json_generation branch 2 times, most recently from 3e49cd8 to 5432855 Compare September 19, 2017 12:22
@akosyakov akosyakov force-pushed the ak/get_rid_of_package_json_generation branch 7 times, most recently from 16bd190 to 44f764c Compare September 20, 2017 06:59
@akosyakov
Copy link
Member Author

akosyakov commented Sep 20, 2017

Just to clarify: we do it to streamline the setup of Theia itself as well as the setup for extension developers and simplify the dynamic extension system.

An extension developer will be able to create package.json using standard tools, like npm init, list extension packages and add @theia/cli as a dev dependency:
https://github.com/theia-ide/theia/blob/44f764c9f195364ace851e88772c6302adef06b1/examples/browser/package.json#L1-L35
After installing such package he will use theia cli tool to manage an application, see npm scripts above. So such command like npx theia browser --port=3001 --root-dir=/home/user/mydir or via npm script yarn start -- --port=3001 --root-dir=/home/user/mydir would be possible.

For the Theia setup, we won't need the package.json generation phase anymore that allows removing some build scripts, simplify root package.json, enable yarn workspaces.

@hexa00
Copy link

hexa00 commented Sep 20, 2017

What is not supported by node 6 in this ?

@akosyakov
Copy link
Member Author

@hexa00 async/await with pure js, bogus cluster module with node 6

@akosyakov
Copy link
Member Author

LTS of node 8 should start in October: https://github.com/nodejs/Release#release-schedule1

@akosyakov akosyakov force-pushed the ak/get_rid_of_package_json_generation branch 11 times, most recently from 6cb3c3c to 287e83f Compare September 22, 2017 09:27
@akosyakov
Copy link
Member Author

akosyakov commented Sep 22, 2017

I would like to merge it if no objections and go on with the dynamic extension system.

@akosyakov akosyakov force-pushed the ak/get_rid_of_package_json_generation branch from aba8cf8 to e36aa55 Compare September 22, 2017 11:38
@akosyakov
Copy link
Member Author

@hexa00 I've merged test:fast and test:slow in single test and adjusted pattern to cover all *.*spec.ts files that there is no special handling for the terminal package and everything is covered
https://github.com/theia-ide/theia/blob/e36aa557bf687ece15a95b44c64e671b57445ea8/dev-packages/ext-scripts/package.json#L26-L27

@hexa00
Copy link

hexa00 commented Sep 22, 2017

I think the coverage gets merged.
OK for the merged tests if we can separate it when need be.

@akosyakov
Copy link
Member Author

@hexa00 yeah, it would be a bit messy with dummy spec files, we should look for the better way generally

@hexa00
Copy link

hexa00 commented Sep 22, 2017

@akosyakov Yes must be a way to treat warnings as warnings rather than errors

@hexa00
Copy link

hexa00 commented Sep 22, 2017

Did you test the yarn.lock ?

@akosyakov
Copy link
Member Author

@hexa00 I fixed the propagation of the error code broken tests should fail the build now

@hexa00
Copy link

hexa00 commented Sep 22, 2017

Testing that now

@akosyakov
Copy link
Member Author

@hexa00 nope, but should work, now lerna does not call install anymore, it is completely done by yarn

@hexa00
Copy link

hexa00 commented Sep 22, 2017

Works fine , thanks! I need to increase that timeout

@hexa00
Copy link

hexa00 commented Sep 22, 2017

Tested yarn.lock seems to work yeee!
(tested by using ^1.7.6 for electron)

@hexa00
Copy link

hexa00 commented Sep 22, 2017

The log output is also so much easier to see!

@akosyakov akosyakov force-pushed the ak/get_rid_of_package_json_generation branch from e36aa55 to bfd249c Compare September 22, 2017 15:57
@akosyakov akosyakov changed the title Get rid of package.json generation Get rid of package.json generation and yeoman generator Sep 22, 2017
Signed-off-by: Anton Kosiakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosiakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosiakov <anton.kosyakov@typefox.io>
Signed-off-by: Anton Kosiakov <anton.kosyakov@typefox.io>
@akosyakov akosyakov force-pushed the ak/get_rid_of_package_json_generation branch 2 times, most recently from 2ffb146 to 03ec13c Compare September 23, 2017 05:33
@akosyakov
Copy link
Member Author

akosyakov commented Sep 23, 2017

I've merged theia-generator into the cli:

  • there are no dependencies on yo and yeoman-generator anymore, it makes windows builds stable
  • there are no yo configs, modules are discovered with require.resolve after installing
  • webpack configs are regenerated each time and added to git ignore
  • moved all dev dependencies to the root package, new should be added there as well

Please review.

Copy link
Contributor

@svenefftinge svenefftinge left a comment

Choose a reason for hiding this comment

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

Looks really good. I would consider the generator to be part of the extension system, but we can move that in another PR.

Signed-off-by: Anton Kosiakov <anton.kosyakov@typefox.io>
@akosyakov akosyakov force-pushed the ak/get_rid_of_package_json_generation branch from 03ec13c to 07d79d3 Compare September 23, 2017 19:12
@akosyakov akosyakov merged commit 0e06afa into master Sep 25, 2017
@akosyakov akosyakov deleted the ak/get_rid_of_package_json_generation branch September 25, 2017 09:25
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