10000 Use Wireit for nimble-components build by m-akinc · Pull Request #1267 · ni/nimble · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Use Wireit for nimble-components build #1267

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
Closed
wants to merge 18 commits into from
Closed

Use Wireit for nimble-components build #1267

wants to merge 18 commits into from

Conversation

m-akinc
Copy link
Contributor
@m-akinc m-akinc commented May 23, 2023 •
edited
Loading

Pull Request

🤨 Rationale

I looked into a couple productivity tools for Innovation Day. I'm adding support for Wireit to manage build caching/dependencies for nimble-components package build.

👩‍💻 Implementation

  • added dev dependency on Wireit
  • converted 6 build-related commands in package.json to use Wireit
  • changed tsc compile to use incremental build (as recommended by Wireit docs)

🧪 Testing

Ran build commands locally

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

@m-akinc m-akinc requested review from rajsite and jattasNI as code owners May 23, 2023 20:15
jattasNI
jattasNI reviewed May 23, 2023
View reviewed changes
@m-akinc m-akinc requested a review from jattasNI May 24, 2023 15:33
rajsite
rajsite reviewed May 24, 2023
View reviewed changes
@m-akinc m-akinc requested a review from rajsite May 24, 2023 17:15
jattasNI
jattasNI reviewed May 25, 2023
View reviewed changes
@m-akinc m-akinc requested a review from jattasNI May 26, 2023 19:08
jattasNI
jattasNI reviewed May 26, 2023
View reviewed changes
Copy link
Contributor
@jattasNI jattasNI left a comment

Choose a reason for hiding this comment

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

Once the build is fixed I'll try it one more time on my dev machine, but it looks great to me! 🎆

@rajsite
Copy link
Member
rajsite commented Jul 25, 2023 •
edited
Loading

I'm gonna move this to draft because it's a bit on the backburner for now and would likely need to be updated to resolve conflicts. Think a good time to revive would be another innovation day.

@m-akinc may be useful to split out the karma stuff in a separate review if that plugin is generally useful for the team. The wire-it stuff is a bigger change I want to have to to double check thoroughly before pushing out to everyone.

@rajsite rajsite marked this pull request as draft July 25, 2023 22:49
@m-akinc m-akinc marked this pull request as ready for review December 14, 2023 00:23
@m-akinc m-akinc changed the title Use Wireit for nimble-components build and support Karma Test Explorer extension Use Wireit for nimble-components build Jan 2, 2024
@rajsite rajsite self-assigned this Feb 28, 2024
rajsite
rajsite reviewed Feb 28, 2024
View reviewed changes
.gitignore
@@ -121,3 +121,6 @@ package-lock.json

Copy link
Member

Choose a reason for hiding this comment

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

As discussed yesterday moving to draft and assigning to myself to take a look in the future.

@rajsite rajsite marked this pull request as draft February 28, 2024 19:21
@rajsite
Copy link
Member
rajsite commented Jul 2, 2024

Closing as unprioritized right now. Improvements to test / re-test execution likely achievable with typescript incremental build caching and switching to vite builder. Full repo build caching with wireit adds a lot of noise / complexity to npm build scripts.

@rajsite rajsite closed this Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rajsite rajsite rajsite left review comments

@jattasNI jattasNI jattasNI approved these changes

@msmithNI msmithNI msmithNI approved these changes

@fredvisser fredvisser Awaiting requested review from fredvisser fredvisser is a code owner

@atmgrifter00 atmgrifter00 Awaiting requested review from atmgrifter00

Assignees

@rajsite rajsite

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@m-akinc @rajsite @jattasNI @msmithNI
0