siggijons / graph-untangler-plugin

Gradle plugin to help untangle your dependencies.
Apache License 2.0
43 stars 1 forks source link

Question: Supporting GitHub CODEOWNERS #2

Open jraska opened 1 year ago

jraska commented 1 year ago

Hey Siggi,

Did you consider supporting GitHub CODEOWNERS with the ownership feature? :)

I see now the input format is a yaml - was there any particular reason for that format? I'm not sure if this yaml is any standard coming from somewhere 🤔

The reason I ask for GitHub CODEOWNERS is that it is somehow a "standard" supported by GitHub and few other tools with specific file format etc. so for some projects (like the ones I'm involved 😛 ) there wouldn't be a need to generate a specific ownership yaml from CODEOWNERS which is used as source of truth.

Thanks for the answer and the plugin 👏

siggijons commented 1 year ago

Hey @jraska 👋

The format is non-standard, but loosely based on Gerrit owners plugin https://gerrit.googlesource.com/plugins/owners/+show/master/config.md

One problem with CODEOWNERS, and part of the reason I didn't use it, is that it doesn't necessarily map to gradle modules since a module can have 0..n owners 💭

Going to take a quick stab at supporting something.

siggijons commented 1 year ago

@jraska could you take a look #6, maybe throw some real data at it and see how it reacts?

jraska commented 1 year ago

Hey @siggijons, thanks for trying it! I completely get the problem with ambiguous module owners as CODEOWNERS is not module based :) even if used like that very often 🤷

I tried the plugin from this branch and I think it struggles with whitespaces

Only a single module got attributed properly in an example input

/module-dir       @team-one  # no owner
/other-module-dir @team-one  # single space, attributed properly
/one-more.        @team-one  # no owner
siggijons commented 1 year ago

Hey @jraska Fixed the regex for space matching and added test 33f21a16f1cc988dc86d8d7551a822c7a835e93a

Can you see if this works better?

jraska commented 1 year ago

Yes, works well now on project I tried. Thanks!