Skip to content
This repository has been archived by the owner on May 1, 2020. It is now read-only.

fix(build): fix issue that extends in ts-config.json property is … #851

Closed

Conversation

alan-agius4
Copy link
Contributor

@alan-agius4 alan-agius4 commented Mar 28, 2017

Short description of what this resolves:

While typescript support extends of another tsconfig, a limitation in validateRequiredFilesExist method doesn't as it's reading the file using fs, rather than getTsConfig which is a wrapper of the TypeScript API,

Changes proposed in this pull request:

  • Impalement getTsConfigAsync method
  • In validateRequiredFilesExist use it rather than readFileAsync for the tsconfig.json

Note: I cannot run Unit Tests

Fixes: #745

@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Mar 28, 2017

Bdw, you be good to enable CircleCI to run on PRs, as well
This can be done in here; https://circleci.com/gh/driftyco/ionic-app-scripts/edit#advanced-settings

@alan-agius4
Copy link
Contributor Author

@danbucholtz any updates?

@danbucholtz
Copy link
Contributor

@alan-agius4,

Can you rebase and submit again? At first glance I would think this would be fine.

Thanks,
Dan

@alan-agius4
Copy link
Contributor Author

@danbucholtz, just did :)

@danbucholtz
Copy link
Contributor

screen shot 2017-04-17 at 1 52 09 pm

I see this. Can you fix? I'll test and merge. Please make sure tests pass as well. I updated so they should 🐱 pass on Windows.

Thanks,
Dan

@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Apr 19, 2017

@danbucholtz all tests are green and rebased.

@danbucholtz
Copy link
Contributor

@alan-agius4,

Can you just resubmit this? Github is not letting me merge still.

Thanks,
Dan

@alan-agius4
Copy link
Contributor Author

@danbucholtz, here's the new PR #910

@alan-agius4 alan-agius4 deleted the feature/fix-ts-config-issues branch April 21, 2017 05:14
@danbucholtz
Copy link
Contributor

Thanks, I'll test and merge tomorrow.

Thanks,
Dan

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.

2 participants