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

Added TypeScriptCompile to CollectedFiles. #1

Merged
merged 1 commit into from
Jul 18, 2016
Merged

Added TypeScriptCompile to CollectedFiles. #1

merged 1 commit into from
Jul 18, 2016

Conversation

dlemstra
Copy link
Contributor

Your comment states that CollectedFiles should collect the files the same way as OctoPack but you did not include @(TypeScriptCompile) (https://github.com/OctopusDeploy/OctoPack/blob/abd0c5d30cfd0572ff066b83436969661b9cdbd4/source/tools/OctoPack.targets#L73) and this patch adds that. You might want to change your comment if you did not include this on purpose.

@LorandBiro
Copy link
Owner

You're right! I remember seeing @(TypeScriptCompile) in the Octopack target, but it looks like I forgot about it. Thank you very much!

@LorandBiro LorandBiro merged commit 19dd53d into LorandBiro:master Jul 18, 2016
@LorandBiro
Copy link
Owner

I was a little confused for a while. I was guessing that @(TypeScriptCompile) contains the output of the TypeScript compiler (which would be a real problem to ignore), but it contains the TypeScript source files. I noticed that the Octopack target contains a flag (OctoPackIncludeTypeScriptSourceFiles) to instruct Octopack whether to include the TypeScript source files or not. It's false by default.

I'm not sure what is exactly the use case. When would someone want to deploy their TypeScript source files?

Anyway, the goal is really to follow the original behavior, so I added the condition. See de52049.

@dlemstra dlemstra deleted the CollectedFiles branch July 21, 2016 19:02
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