Skip to content

Conversation

@liucijus
Copy link
Collaborator

@liucijus liucijus commented Mar 5, 2021

Implements #1240

@liucijus liucijus requested a review from ittaiz as a code owner March 5, 2021 10:50
@google-cla google-cla bot added the cla: yes label Mar 5, 2021
@liucijus liucijus added the dep-tracking Strict/unused deps, label collections related issues label Mar 5, 2021

all_strict_deps_patterns = ctx.attr.dependency_tracking_strict_deps_patterns

strict_deps_include_patterns = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe extract a function to partition patterns into include/exclude lists

return ctx.toolchains["@io_bazel_rules_scala//scala:toolchain_type"].unused_dependency_checker_mode

def _is_target_included(target, includes, excludes):
if len([exclude for exclude in excludes if target.startswith(exclude)]) > 0:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe instead of building list and checking length you could use loop with an if? For example

for exclude in excludes:
  if target.startswith(exclude):
    return False

Or maybe if any([target.startswith(exclude) for exclude in excludes]) but I guess you would building new list each time however it might not be an issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm leaning towards simple loop

Copy link
Collaborator

@simuons simuons left a comment

Choose a reason for hiding this comment

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

LGTM. Just couple minor comments/suggestions.

@ittaiz ittaiz merged commit 68525d9 into bazel-contrib:master Mar 10, 2021
wisechengyi pushed a commit to twitter-forks/rules_scala that referenced this pull request Mar 11, 2021
* Add target prefix filters support for strict/unused deps

* Add dependecy tracking filtering docs

* Remove todo comment

* Extract deps pattern partition function, optimize pattern macthing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes dep-tracking Strict/unused deps, label collections related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants