You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
codeowners(
name = "my_team",
patterns = [
"/application-a/",
"/application-b/",
],
teams = [
"@tink-ab/my-team",
],
)
I just had a case where I removed all files in the application-a, including the directory. However, the generate_codeowners(...) target happily executed. It would nice if it would have failed telling me that I should removed patterns not patching anything (and possibly support disabling this check if files can be checked in dynamically or something).
The text was updated successfully, but these errors were encountered:
Hmm, so really what you're asking for is validation that the arbitrary pattern entered by a user of the rule matches paths in the workspace.
This gets a little trickier when considering that different platforms infer the patterns differently, e.g. Gitlab to Github have slightly different ownership syntax.
@zegl do you have thoughts about handling this cleanly? One thought I have is that we could do a little refactoring to build the workflow around something like:
... which would be a little more consistent with other rules and would effectively accomplish this goal. There are some downsides, though:
Our rule would get a little more complex as it would have to merge all the runfiles from the inputs before building the ownership strings.
We'd be removing arbitrary pattern matching from this library. This mainly affects usability of the rule and the verbosity of the generated file (which will now always have explicit paths and not patterns).
We wouldn't be able to define ownership for nonexistent paths (like we currently can), eg. a single entry for global or default ownership would no longer be possible.
I'd personally be okay with these trade-offs, but only because my teams have a hard requirement for ensuring owners are defined for paths at review time, and I have a test that ensures all paths in my repo are covered by an owners entry.
Rule:
I just had a case where I removed all files in the
application-a
, including the directory. However, thegenerate_codeowners(...)
target happily executed. It would nice if it would have failed telling me that I should removed patterns not patching anything (and possibly support disabling this check if files can be checked in dynamically or something).The text was updated successfully, but these errors were encountered: