Allow setting default arguments with `./pants tailor`

[Eric-Arellano]

@cognifloyd shared that he wants to be able to set skip_pylint by default. It's tedious to manually add that.

The modeling should probably be a dict option?

[tailor.defaults]
python_library = { skip_pylint = true }
python_tests = { skip_pylint = true }

(Unclear if we let you use TOML primitive values like true, or we expect you to string escape all values like "True". I think TOML values should work out fine.)

[Eric-Arellano]

PS: I love how native TOML dict support makes this so much more ergonomic. And how that support was entirely prompted by refactoring of our options code that was unrelated to that feature.

[cognifloyd]

When we start allowing tailor to edit targets, I wonder how the config for that will look. [tailor.defaults] would only apply when adding a new target to a BUILD file right? Not when editing an existing target?

[Eric-Arellano]

That's a great question. I've put no design work into what the UX would be for editing existing targets, outside of hardcoded edits like ./pants tailor --fix=file_addresses. I don't know what generic editing support would look like.

But yeah, I think it makes sense for this option to only impact new targets. We should probably have a dedicated option like [tailor.edit_defaults] for edits, as I can imagine wanting them to be different. Wdyt?

[cognifloyd]

What about starting off with something more verbose here: [tailor.new_target_defaults]

[kaos]

Slightly off topic, but:

It would be cool if tailor could become a generic tool for batch update BUILD files. Something like:

./pants tailor --batch-edit="set skip_pylint=True for python_library(tag='legacy')" \
    tree/to/update  \
    and/this/tree

Where the argument to --batch-edit is a small DSL for managing BUILD files. Perhaps way to niche and rare to be worth it, but would be very powerful, esp in large code bases.

[cognifloyd]

Niche? No. Batch edit would be extremely useful for gradual pants adoption in existing code bases.

[Eric-Arellano]

@cognifloyd how so for BUILD files you already have? I see the use for generating the initial files, but don't see it as much for after that.

For example: you set skip_pylint=True at first, but want to chip away at that to enable Pylint for everything. My mental model is you'd do it incrementally, a couple BUILD files at a time per PR. Not in one big change. So, hand editing isn't a big deal.

[kaos]

Or for refactoring tasks..

[cognifloyd]

That was a new feature assuming I had already created the build files.

So there's a really good chance that you'll run tailor and then while testing you decide, "nope it's not worth running this tool on this tree/slice of the repo". And then you've got to go back and edit all of the BUILD files manually.

I learn things gradually, so I would run tailor without any options configured, and then start tuning it.

Also, if I add a new tool (enable a new backend) then I'm going to edit existing targets to skip the tool in the appropriate directories.

If tailor is only good for new targets, then it seems you assume I have perfect foresight on what the defaults should be before I run tailor, but I don't. So anything that can speed up mass target edits would be amazing I think.

Now I'm up way too late, I can't tell if my comments are harsh and I don't want to be mean, so I'll wait till tomorrow before I comment more.

[Eric-Arellano]

Not harsh at all, very useful. Thank you! That makes sense.

[AlexTereshenkov]

Slightly off topic, but:

It would be cool if tailor could become a generic tool for batch update BUILD files. Something like:

./pants tailor --batch-edit="set skip_pylint=True for python_library(tag='legacy')" \
    tree/to/update  \
    and/this/tree

Where the argument to --batch-edit is a small DSL for managing BUILD files. Perhaps way to niche and rare to be worth it, but would be very powerful, esp in large code bases.

FWIW this functionality is already provided by buildozer. I appreciate it may be nice to have a control over this type of functionality within Pants, but this means more code for the functionality that has already been implemented and tested thoroughly.

@cognifloyd do you think using this tool would satisfy your requirements for batch editing? There's https://blog.pantsbuild.org/updating-pants-build-files-programmatically/ where you can see practical examples of running this over Pants BUILD files.

I guess the use case is that in a legacy codebase, you don't want to your targets to run, say, pylint on, so you don't enable the corresponding backend in the pants.toml. Then at some point you enable it, but don't want to run it for all the targets, maybe a project or two (those brave people who are ready to fix all the warnings!). So you now have to edit the rest of BUILD files to put skip_pylint=True. Once the project owners are ready to fix the warnings, you'd remove that field. And the process goes on... When creating a new target, though, I am not sure why one would want to disable a linter/formatter as it's new code.

However, I totally get if one would want to set arbitrary properties e.g. python_sources.interpreter_constraints to be something custom to apply for all new targets, so that one doesn't have to go and add it manually for all targets. However, once the targets are created with defaults, and then you later discovered that you had to adjust python_sources.interpreter_constraints, you won't likely be able to delete the BUILD files and re-run tailor as you may have made some manual changes you may not want to lose. So we are back again to batch editing BUILD files, for which one can use buildozer.

[cognifloyd]

The __defaults__ feature satisfies much of this use case.

But, I still find myself doing a grep ... **/BUILD (I love the ** glob in zsh) for the targets that implement whatever I need to edit, and then edit each of them.

If bulldozer provides a way to do mass edits, then we could just wrap it in a pants goal, and pants can use it to edit the files.