-
Notifications
You must be signed in to change notification settings - Fork 17
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
Package benchmarking #18
Comments
IMO Nanosoldier probably won't scale to that. There is some idle capacity on Nanosoldier, but I think that would be better directed to running automatically on more Base PR's (ones where no one remembers to manually trigger it) when idle. |
Yeah, I agree with this. Nanosoldier's hardware should be dedicated to Base testing. Another aspect of this issue, though, might be repo/org maintainers who have their own hardware, and would like to set up the Nanosoldier benchmarking service on that hardware. This is almost feasible; Nanosoldier is pretty close to being generic enough already. The real dev effort required there would go towards better testing for this package (which is currently dismal) and documentation. Besides simply not having the time, I've been holding off on these changes until BenchmarkTools has the ability to quantify statistical significance of performance changes, rather than just effect size...I'm weary about how noisy random people's hardware will be. |
In case this issue ever becomes viable (i.e. we ever get more hardware + I get the opportunity to refactor this package the way I eventually want to), it would be really cool for Nanosoldier to be a GitHub Integration a la @simonbyrne's attobot. GitHub's public integration API didn't exist (or maybe wasn't documented?) when I originally developed Nanosoldier, but now it seems like the way to go for these sorts of things. |
This may be related to #1, but just curious about your thoughts on whether packages could do their own performance-tracking, or whether Nanosoldier's bandwidth is unlikely to scale appropriately.
The text was updated successfully, but these errors were encountered: