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

Add an option to black-primer to run on a sample of projects #2578

Closed
Smlep opened this issue Oct 30, 2021 · 4 comments
Closed

Add an option to black-primer to run on a sample of projects #2578

Smlep opened this issue Oct 30, 2021 · 4 comments
Labels
C: maintenance Related to project maintenance, e.g. CI, testing, policy changes, releases T: enhancement New feature or request

Comments

@Smlep
Copy link

Smlep commented Oct 30, 2021

Hi 👋

Is your feature request related to a problem? Please describe.

Here

 # TODO: Offer more options here
 # e.g. Run on X random packages etc.

is written that adding more options to black-primer so that only a part of all the projects in config could be used should be done.

Describe the solution you'd like

Add an option --random-packages-count N, N being an int.
Then, only run the tests on N of the packages in primer.json.
If N is greater than the count of packages in primer.json, every package should be used.

Additional context

I'd be happy to submit a PR to implement this!

@Smlep Smlep added the T: enhancement New feature or request label Oct 30, 2021
@Smlep Smlep changed the title Add options to black-primer to run on a sample of projects Add an option to black-primer to run on a sample of projects Oct 30, 2021
@cooperlees
Copy link
Collaborator

2 things here:

  • I'd only like to add features that have a use case / need - What's the use case here? Would you be using it or just doing it cause you saw the comment?
  • @ichard26 is looking to incorporate all the black-primer features into a new tool that "borrows" back some features mypy-primer does better so this tool will be merging with that and prob be deleted etc.

@Smlep
Copy link
Author

Smlep commented Oct 30, 2021

The main use case in my opinion would be to be able to run the black-primer faster since it currently has to run on all projects (even with --long-checkouts disabled) and it takes a considerate amount of time (about 16 minutes on my laptop, but I have a pretty slow internet connection).

I'd like to run primer only on a few projects to be able to test black changes against real-life code without having to wait a dozen minutes.

But well, if this tool is supposed to be deleted/fully refactored soon, then yeah this feature should not be implemented and you can close this 👌 (and remove the comments).

@ichard26 ichard26 added the C: maintenance Related to project maintenance, e.g. CI, testing, policy changes, releases label Dec 26, 2021
@ichard26
Copy link
Collaborator

ichard26 commented Dec 26, 2021

OK yeah, to be transparent I am intending to fully replace black-primer with diff-shades (I'm open to renaming it) eventually. As far as I can tell it's better than primer in most ways, highlight features include:

The main downside of diff-shades is that it's certainly less robust than primer given it uses more internal APIs of Black to provide this deeper level of integration. Also Jupyter notebooks aren't supported right now (and nor would markdown documents if #2721 was landed). Finally more code documentation wouldn't hurt. Once I resolve these issues I'll start to look into replacing all usages of primer.

Anyhow while I wouldn't outright reject a sample feature for diff-shades's analyze command, I'd argue it won't be needed as much as I'm planning to integrate diff-shades into CI (see GH-2725) -- this is notable because one feature of the integration is automated PR comments that describe the impact of the PR in question in a quantifiable manner. Here's some examples:

Hopefully this makes it less necessary to run primer or diff-shades locally (which is a win given how slow it can be, it takes 25-35 minutes on my laptop! It's actually a time saver to first compile Black with mypyc and then run diff-shades1).

Footnotes

  1. this is ignoring cloning which takes another 10 minutes 🙃

@ichard26 ichard26 reopened this Dec 26, 2021
@felix-hilden
Copy link
Collaborator

We've now stopped using primer and the deprecation plan is discussed here #2821.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: maintenance Related to project maintenance, e.g. CI, testing, policy changes, releases T: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants