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

refactor: Reworked build system #25

Closed
wants to merge 5 commits into from
Closed

refactor: Reworked build system #25

wants to merge 5 commits into from

Conversation

rschristian
Copy link
Contributor

I looked into doing this as I was having issues wrangling Microbundle into outputting types in #23. The issue is outlined a bit there.

This would not only be a solution to that problem but also has the potential to be an improvement outside of that. Reduces surface area of maintenance and locally I'm seeing builds take half the time.

@omgovich
Copy link
Owner

omgovich commented Aug 24, 2020

Works like a charm! ❤️

@omgovich
Copy link
Owner

Ready to merge and deploy on your command)

@rschristian
Copy link
Contributor Author

rschristian commented Aug 24, 2020

Should be ready to merge. I don't think it'll need a deploy though, as nothing is functionally different.

I did realize that I neglected to copy over the name flag for the new scripts, so that's been fixed. I just copied it over as you had it, not sure if you'd like a different format with that.

Edit: Thought flags could just be overridden. Had to add an extra command to accommodate the name flag.

Comment on lines -17 to -24
# packages
/hex
/hsl
/hslString
/hsv
/rgb
/rgbString
/HexInput
Copy link
Owner

@omgovich omgovich Aug 24, 2020

Choose a reason for hiding this comment

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

Discussion:

I feel that this PR is going to make the top-level repo structure more complex.
And that'll make the barrier to entry way higher.
I just thought that keeping so many folders in repo root isn't the best idea.

It seems to me that we could combine build-packages.js solution and the one you did. I mean to create the same package.json, dist subfolders, run the same command etc automatically using one JS-script. I'm going to create a separate PR to try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. Though I still think this will just move the complexity into your tooling instead. It'll also be slower, though depending on what you're used to, might not be important.

You'll just need to first generate a valid package.json, then run the Microbundle in that directory, rather than the other way around.

@omgovich
Copy link
Owner

Going to close this PR. These ideas and concepts helped us to improve building process in #26.
Thanks @ryanchristian4427 ❤️

@rschristian
Copy link
Contributor Author

#26 is the successor

@rschristian rschristian deleted the refactor/buildProcess branch August 26, 2020 16:51
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