Skip to content

fix for #14467 - fix(@schematics/angular): webworker snippet should not generate to module #14468

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 21, 2019
Merged

fix for #14467 - fix(@schematics/angular): webworker snippet should not generate to module #14468

merged 2 commits into from
May 21, 2019

Conversation

@santoshyadavdev
Copy link
Contributor

@santoshyadavdev santoshyadavdev commented May 19, 2019 •
edited
Loading

Fixes #14467

@santoshyadavdev santoshyadavdev force-pushed the fix(@schematics/angular)--web-worker-snippet-should-be-generated-to-component branch 2 times, most recently from 2765fb6 to 7d687fe Compare May 19, 2019 17:28
@alan-agius4 alan-agius4 requested a review from filipesilva May 20, 2019 07:25
@alan-agius4
Copy link
Collaborator

alan-agius4 commented May 20, 2019

@santoshyadav198613, thanks for this. Can you kindly fix the fixes in the commit message as well?
Thanks.

@santoshyadavdev
Copy link
Contributor Author

santoshyadavdev commented May 20, 2019

Sure @alan-agius4 , will do thanks.

filipesilva
filipesilva approved these changes May 20, 2019
View reviewed changes
Copy link
Contributor

@filipesilva filipesilva left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@filipesilva filipesilva added the target: patch This PR is targeted for the next patch release label May 20, 2019
alan-agius4
alan-agius4 reviewed May 20, 2019
View reviewed changes
@alan-agius4 alan-agius4 added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label May 20, 2019
@santoshyadavdev santoshyadavdev force-pushed the fix(@schematics/angular)--web-worker-snippet-should-be-generated-to-component branch 2 times, most recently from 118ef01 to 5f99db2 Compare May 20, 2019 14:02
@santoshyadavdev santoshyadavdev changed the title fix(@schematics/angular): webworker snippet should generate to component fix(@schematics/angular): webworker snippet should not generate to module May 20, 2019
alan-agius4
alan-agius4 reviewed May 20, 2019
View reviewed changes
alan-agius4
alan-agius4 requested changes May 20, 2019
View reviewed changes
Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

See above.

@alan-agius4 alan-agius4 removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label May 20, 2019
@IgorMinar IgorMinar added this to the 8.0 milestone May 20, 2019
@IgorMinar IgorMinar changed the title fix(@schematics/angular): webworker snippet should not generate to module fix for #14467 fix(@schematics/angular): webworker snippet should not generate to module May 20, 2019
@IgorMinar IgorMinar changed the title fix for #14467 fix(@schematics/angular): webworker snippet should not generate to module fix for #14467 - fix(@schematics/angular): webworker snippet should not generate to module May 20, 2019
@alan-agius4 alan-agius4 force-pushed the fix(@schematics/angular)--web-worker-snippet-should-be-generated-to-component branch from 5f99db2 to 50ceb4d Compare May 21, 2019 06:08
@googlebot
Copy link

googlebot commented May 21, 2019

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

alan-agius4
alan-agius4 approved these changes May 21, 2019
View reviewed changes
Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

Hi @santoshyadav198613, sorry to barge in your PR and pushing directly. But we need to get this merged ASAP for the version 8 final.

Thanks a lot for your contribution.

//cc @alexeagle, this is a go!

alan-agius4
alan-agius4 reviewed May 21, 2019
View reviewed changes
packages/schematics/angular/web-worker/index_spec.ts
inlineStyle: false,
inlineTemplate: false,
routing: false,
routing: true,
Copy link
Collaborator

@alan-agius4 alan-agius4 May 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Changing this value, will ensure that the improved logic is tested.

@santoshyadavdev
Copy link
Contributor Author

santoshyadavdev commented May 21, 2019

Hi @santoshyadav198613, sorry to barge in your PR and pushing directly. But we need to get this merged ASAP for the version 8 final.

Thanks a lot for your contribution.

//cc @alexeagle, this is a go!

thanks @alan-agius4 , sorry was not feeling well, so was not able to make changes last night. But happy that I was able to find the issue.

@mgechev mgechev added cla: yes and removed cla: no labels May 21, 2019
@googlebot
Copy link

googlebot commented May 21, 2019

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@mgechev mgechev merged commit 0d493cb into angular:master May 21, 2019
@fareshan fareshan mentioned this pull request Jul 16, 2019
Closed
@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented Sep 13, 2019

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@alan-agius4 alan-agius4 alan-agius4 approved these changes

+1 more reviewer

@filipesilva filipesilva filipesilva approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

8.0

Development

Successfully merging this pull request may close these issues.

Code generated for webWorker in incorrect file

6 participants

@santoshyadavdev @alan-agius4 @googlebot @filipesilva @IgorMinar @mgechev