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

Core path update #49

Merged
merged 5 commits into from
Apr 17, 2020
Merged

Core path update #49

merged 5 commits into from
Apr 17, 2020

Conversation

pepelsbey
Copy link

@shower/core is now released as v3.0.0 stable, so I’m currently moving the rest of the packages to the @shower organization. To publish the @shower/shower package I need to update the cli with the new core path first.

Would you mind checking my changes and releasing the new cli version? :)

@mrdimidium
Copy link

Hi. Thank you for your contribution.

I'm at work right now, but I'll check and release the update today.

Copy link

@mrdimidium mrdimidium left a comment

Choose a reason for hiding this comment

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

Updating the path in the template is not enough. You need to update the regex in build script.

This is it:

/(<script src=")(node_modules\/@shower\/core\/)(shower.min.js"><\/script>)/g,

If you don't want to get into the code, I can finish it myself tonight.

@pepelsbey
Copy link
Author

If you don't want to get into the code, I can finish it myself tonight

Huh, too late. Done :)

@mrdimidium
Copy link

Oh, and the filename too, please :)

'shower.min.js'

@pepelsbey
Copy link
Author

Done.

It kinda makes you understand that it’s a bit too much hard-coded ;)

@mrdimidium
Copy link

Yes, I agree that it is worth making this dependency more flexible.

@mrdimidium mrdimidium merged commit 2c34567 into master Apr 17, 2020
@mrdimidium
Copy link

I published version 0.3.0 in npm, with your PR.

@pepelsbey pepelsbey deleted the core-update branch April 17, 2020 19:25
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.

3 participants