Skip to content

Conversation

yushan26
Copy link
Contributor

Implement a new directive to ignore targets in an existing BUILD.bazel file to prevent gazelle from modifying or cleaning up existing python rules.

# gazelle:python_ignore_target //foo/bar:baz
# gazelle:python_ignore_target //foo/bar:baz_test
# gazelle:python_ignore_target //a/b/c/d:e

Fixes: #3105

Copy link
Collaborator

@dougthor42 dougthor42 left a comment

Choose a reason for hiding this comment

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

Thanks! Two main requests:

  1. Add test cases
  2. Update the docs in gazelle/docs/directives.md

I'd also like to get feedback from @aignas: As mentioned in the feature request I opened (#3105), this is already possible via Gazelle # keep comments like so:

# keep: gazelle will not touch this entire target
def py_library( ... )

My question is: do we want to make this part of rules_python_gazelle_plugin? Or would this be more suitable as part of Gazelle core? Or maybe it's one of those "there's already a way to do X so we don't want to implement a second way to do X" things?


// IgnoreTarget returns whether the target should be ignored.
func (c *Config) IgnoreTarget(target string) bool {
_, ignores := c.ignoredTargets[target]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder... should we support wildcards/globbing/regex? And how about the $python_root$ replacement?

# gazelle:python_ignore_target //$python_root$/foo/bar:*_foo

py_library(name = "bar_foo", ...)  # ignored
py_library(name = "bar_bar", ...)  # not ignored
py_library(name = "foo_foo", ...)  # ignored

Regex is probably too complex, especially if we want to support the $python_root$ replacement literal. But simple glob-like matching might be OK. There's precedence for globs, too: TestFilePattern uses it and we can make use of matchesAnyGlob.

}

// IgnoreTarget returns whether the target should be ignored.
func (c *Config) IgnoreTarget(target string) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's call this IsIgnoredTarget.

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

Successfully merging this pull request may close these issues.

Feat: Gazelle directive # gazelle:python_ignore_target target
3 participants