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

Fixed check for missing script #198

Merged
merged 2 commits into from
Jul 4, 2021
Merged

Fixed check for missing script #198

merged 2 commits into from
Jul 4, 2021

Conversation

kmalakoff
Copy link
Contributor

@kmalakoff kmalakoff commented Jul 4, 2021

Description

There was a check for tag, but it should have been for asset. The plugin crashed the first time I used it and it looks like a simple logic error.

How has this been tested?

I ran your tests and they passed

Types of changes

  • New feature - feat
  • Bug fix - fix
  • Refactor - refactor
  • Test cases - test
  • Other(s):

Remarks

N/A

@icelam icelam changed the base branch from master to develop July 4, 2021 15:11
@icelam
Copy link
Owner

icelam commented Jul 4, 2021

Hi, @kmalakoff. Thanks for submitting this PR 😊 This really is a careless mistake that I couldn't have spot it out for over an year! I guess my test case isn't really covering the whole scenario, however I didn't manage to find a webpack configuration that can reproduce the error. I would be grateful if you could share me your webpack configuration, so that I can enhance my tests.

@icelam icelam merged commit e9f9f26 into icelam:develop Jul 4, 2021
@kmalakoff
Copy link
Contributor Author

No worries!

There's an example here, but it is is complicated to extract into a nice little test case.

Thank you for the merge and re-publish. I appreciate your efforts

@icelam
Copy link
Owner

icelam commented Jul 5, 2021

Thanks for sharing the information. I will take a look and see how I can reproduce the issue.

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.

None yet

2 participants