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

fix(build-main): move ng dependencies to ROOT of Stark. Fix issue in build process. Rename tsconfig #363

Conversation

SuperITMan
Copy link
Member

@SuperITMan SuperITMan commented May 8, 2018

Move ng dependencies and rxjs dep into the ROOT to avoid install of those shared in multiple
packages.
Rename tsconfig inside packages to tsconfig-build and remove root tsconfig.json file to
make IntelliJ working correctly

ISSUES CLOSED: #361, #362

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[X] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #361, #362

What is the new behavior?

Fix build process + Avoid repetition of dependencies in the project.

Does this PR introduce a breaking change?

[ ] Yes
[X] No

Other information

@dsebastien dsebastien added this to the 10.0.0-alpha.3 milestone May 8, 2018
@SuperITMan SuperITMan force-pushed the feature/move-ng-dependencies branch from 569028f to 98d294c Compare May 8, 2018 08:23
@SuperITMan
Copy link
Member Author

Apparently, there is an issue with tslint. I'll fix that now :)

@SuperITMan SuperITMan force-pushed the feature/move-ng-dependencies branch from 98d294c to c02254b Compare May 8, 2018 09:01
@coveralls
Copy link

coveralls commented May 8, 2018

Coverage Status

Coverage remained the same at 95.594% when pulling c7fecdb on SuperITMan:feature/move-ng-dependencies into f9f3442 on NationalBankBelgium:master.

@@ -64,17 +56,18 @@
"@angular/compiler-cli": "5.x",
"@angular/core": "5.x",
"@angular/platform-browser": "5.x",
"@angular/platform-browser-dynamic": "5.x"
"@angular/platform-browser-dynamic": "5.x",
"rxjs": "5.x"
Copy link
Collaborator

Choose a reason for hiding this comment

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

angular/router is also needed in Stark Core, could you please add it?

Copy link
Member Author

Choose a reason for hiding this comment

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

True, I forgot this. It's added now

@@ -64,17 +56,18 @@
"@angular/compiler-cli": "5.x",
Copy link
Collaborator

@christophercr christophercr May 8, 2018

Choose a reason for hiding this comment

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

Are we sure we need the angular/compiler-cli in Stark Core? and angular/compiler as well?

Aren't they part of Stark Build instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure it's needed. Currently, they are part of the stark-build and stark-core.

Copy link
Collaborator

Choose a reason for hiding this comment

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

To me, we should remove them unless they are really needed. So if you remove them and everything works just fine...

Copy link
Member Author

Choose a reason for hiding this comment

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

For sure, because the deps will be available in the root :)
But ok, I'll remove those peerDeps from stark-core

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

Choose a reason for hiding this comment

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

We should write some docs about the structure of our tsconfig files: #364

@SuperITMan SuperITMan force-pushed the feature/move-ng-dependencies branch from c02254b to 41a998b Compare May 8, 2018 09:44
…build process. Rename tsconfig

Move ng dependencies and rxjs dep into the ROOT to avoid install of those shared in multiple
packages.
Rename tsconfig inside packages to tsconfig-build and remove root tsconfig.json file to
make IntelliJ working correctly

ISSUES CLOSED: #361, #362
@SuperITMan SuperITMan force-pushed the feature/move-ng-dependencies branch from 41a998b to c7fecdb Compare May 8, 2018 10:12
@dsebastien dsebastien merged commit 5502026 into NationalBankBelgium:master May 8, 2018
@SuperITMan SuperITMan deleted the feature/move-ng-dependencies branch May 16, 2018 15:28
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.

all: move ng dependencies and rxjs dependency in the root of the project
5 participants