Skip to content

fix(add): add handling for topScope#796

Merged
evenstensberg merged 4 commits intowebpack:masterfrom
rishabh3112:fix/add
Mar 19, 2019
Merged

fix(add): add handling for topScope#796
evenstensberg merged 4 commits intowebpack:masterfrom
rishabh3112:fix/add

Conversation

@rishabh3112
Copy link
Member

What kind of change does this PR introduce?

BugFix For #795

Did you add tests for your changes?
No
If relevant, did you update the documentation?
No
Summary

#795
Does this PR introduce a breaking change?

No
Other information

@webpack-bot
Copy link

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@misterdev
Copy link
Contributor

Hi @rishabh3112, great job, I confirm the error is gone.

I've copied packages/generators/add-generator.ts in my local fork and build it. When I run add on Windows it adds the line 2 times to webpack.config.js:

bugrishab

Does it happens to you? If not, I'll try again after cleaning my folders 👍

@webpack-bot
Copy link

@rishabh3112 Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@evenstensberg Please review the new changes.

@ghost

This comment has been minimized.

rephrase question
@rishabh3112
Copy link
Member Author

@misterdev Thanks for reporting 💯
This came to be another error with how transformations are handled internally. Fixed it and it worked for me now. You can check again.

Have rephrased question @evenstensberg & @misterdev 👍

@misterdev
Copy link
Contributor

Works now, well done 👍

Copy link
Contributor

@sendilkumarn sendilkumarn left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@rishabh3112 Do you mind adding a test for this?

@rishabh3112
Copy link
Member Author

Hi @sendilkumarn, Can you guide me here? Currently, we are not testing any of the generators ( also smoke tests for scaffold functions). I would love to add them!

@evenstensberg
Copy link
Member

We don't really have a good way to test scaffolds @sendilkumarn

Copy link
Member

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

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

lgtm, nice job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants