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

Option to calculate closure impact #36

Open
timokau opened this issue Jan 12, 2019 · 3 comments
Open

Option to calculate closure impact #36

timokau opened this issue Jan 12, 2019 · 3 comments
Labels
enhancement New feature or request

Comments

@timokau
Copy link

timokau commented Jan 12, 2019

The PR template recommends

* Determined the impact on package closure size (by running `nix path-info -S` before and after

It would be nice to have nix-review do that automatically. This may require a second rebuild, so maybe it should be disabled by default. Then again in 99% of the cases the second rebuild would be quick. Maybe it would even be possible to time the changed build first and then decide based on that weather or not to determine closure size.

@Mic92
Copy link
Owner

Mic92 commented Jan 16, 2019

Seems like a good idea. I would accept an pull request adding option that performs this. Making the selection of attributes where this calculation is performed, would be reasonable.

@davidak
Copy link

davidak commented Dec 4, 2019

This would be helpful!

I had created a shell script to calculate the impact. This can be used to copy the logic.

NixOS/nixpkgs#51368

Making the selection of attributes where this calculation is performed, would be reasonable.

an option is good, but i would prefer also a sane default, like getting the name from the commit title

in most cases the former version can be fetched from hydea. maybe we can just get the size without getting the whole binary?

when a binary is not available and the former build took long, the script could ask if it should build a second time and warn that it could take long

@Mic92
Copy link
Owner

Mic92 commented Dec 5, 2019

Asking hydra seems like a good option if that was possible. I could imagine to just display that by default after the other packages have been built.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants