Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Include sub modules #9

Merged
merged 10 commits into from
Nov 29, 2018
Merged

Include sub modules #9

merged 10 commits into from
Nov 29, 2018

Conversation

blackbaud-conorwright
Copy link
Contributor

Resolves: #6
Docs: N/A

@codecov-io
Copy link

codecov-io commented Nov 14, 2018

Codecov Report

Merging #9 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #9      +/-   ##
==========================================
+ Coverage   99.67%   99.67%   +<.01%     
==========================================
  Files          18       18              
  Lines         309      311       +2     
  Branches       37       37              
==========================================
+ Hits          308      310       +2     
  Misses          1        1
Impacted Files Coverage Δ
...ector-action/list-column-selector-action.module.ts 100% <100%> (ø) ⬆️
...ic/modules/list-view-grid/list-view-grid.module.ts 100% <100%> (ø) ⬆️
...or-action/list-column-selector-action.component.ts 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96c11e9...b166621. Read the comment docs.

@Blackbaud-AlexKingman Blackbaud-AlexKingman self-assigned this Nov 15, 2018
Copy link
Contributor

@Blackbaud-AlexKingman Blackbaud-AlexKingman left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Blackbaud-SteveBrush Blackbaud-SteveBrush left a comment

Choose a reason for hiding this comment

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

It seems that I should be able to write my app-extras.module.ts file like this, and it would work:

import {
  NgModule
} from '@angular/core';

import {
  SkyListModule,
  SkyListToolbarModule
} from '@skyux/list-builder';

import {
  SkyListViewGridModule
} from './public';

@NgModule({
  exports: [
    SkyListModule,
    SkyListToolbarModule,
    SkyListViewGridModule
  ]
})
export class AppExtrasModule { }

Is that correct?

@blackbaud-conorwright
Copy link
Contributor Author

@Blackbaud-SteveBrush Yes, that should work as long as you only use what they provide. However I don't think you can realistically use the SkyListViewGridModule without using SkyGridModule

Here is the one I used for testing:


@NgModule({
  exports: [
    SkyAlertModule,
    SkyAvatarModule,
    SkyErrorModule,
    SkyKeyInfoModule,
    SkyModalModule,
    SkyListModule,
    SkyListViewGridModule,
    SkyWaitModule,
    SkyListSecondaryActionsModule,
    SkyListToolbarModule,
    SkyGridModule,
    SkyListPagingModule
  ]
})
export class AppExtrasModule { }

Is yours not working? I'll try to reduce mine down and see

@blackbaud-conorwright
Copy link
Contributor Author

blackbaud-conorwright commented Nov 20, 2018

@Blackbaud-SteveBrush I have mine working with just:


@NgModule({
  exports: [
    SkyListModule,
    SkyListViewGridModule,
    SkyListToolbarModule,
    SkyGridModule
  ]
})
export class AppExtrasModule { }

You can see it here: https://github.com/blackbaud-conorwright/skyux-spa-my-test

I presume your issue is not having SkyGridModule, but I can see a good case for including that in the exports of SkyListViewGridModule. Though it's weird since it isn't in that lib ;p Do you think we should include that in the exports of SkyListViewGridModule?

@Blackbaud-SteveBrush Blackbaud-SteveBrush self-assigned this Nov 26, 2018
@Blackbaud-SteveBrush
Copy link
Member

The following app-extras.module.ts file will fail to serve (notice the barrel import for SkyListViewGridModule):

import {
  NgModule
} from '@angular/core';

import {
  SkyGridModule
} from '@skyux/grids';

import {
  SkyListModule,
  SkyListToolbarModule
} from '@skyux/list-builder';

import {
  SkyListViewGridModule
} from './public';

@NgModule({
  exports: [
    SkyGridModule,
    SkyListModule,
    SkyListToolbarModule,
    SkyListViewGridModule
  ]
})
export class AppExtrasModule { }

@Blackbaud-SteveBrush
Copy link
Member

@blackbaud-conorwright Looks like we have a few linting errors during the build.

@blackbaud-conorwright
Copy link
Contributor Author

@Blackbaud-SteveBrush Ah yeah, I meant to move those up, but never pasted them ;p
back in now

@Blackbaud-SteveBrush Blackbaud-SteveBrush merged commit 8be101c into master Nov 29, 2018
@Blackbaud-SteveBrush Blackbaud-SteveBrush deleted the include-sub-modules branch November 29, 2018 18:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants