flowwer-dev / pull-request-stats

Github action to print relevant stats about Pull Request reviewers
MIT License
366 stars 79 forks source link

Feature: Add option to exclude users from stats #25

Closed fo2rist closed 1 year ago

fo2rist commented 2 years ago

Would be nice to be able to exclude inactive users and bots completely.

P.S. I had to fork you action because having telemetry was not acceptable for a project I'm working on, but I'd be happy not to maintain the fork anymore if you ever consider removing Mixpanel dependency. You can take implementation of this feature from this commit and this fix. Thanks for your work!

manuelmhtr commented 2 years ago

Hi @fo2rist, thanks for your suggestions.

About excluding users, may I ask the use case? (I guess there are some collaborators who leave, but they still appearing in the stats for a while, is that the problem?)

About mixpanel, I recently added an option to prevent data from being send to external sources. The dependency still there but it's useless, does it works for for? If it doesn't, may I ask why? I need to understand to improve the privacy of the project.

Thanks

fo2rist commented 2 years ago

Hey!

About excluding users, may I ask the use case?

We have collaborators who should not be counted of two types:

The dependency still there but it's useless, does it works for for? If it doesn't, may I ask why?

Main reason is that while the dependency is there it's impossible to guarantee it won't fire accidentally due to a bug, or change in the default value of the parameter one day (the addition of the parameter recently is the example of such case). That means we have to lock the version, and then review the change-log every time we upgrade the action, which is impractical. Another option is to lock and never upgrade to save time, but we wanted a couple of features (like the one above) to be added.

So basically the presence of the code that can send something by mistake was the biggest concern. Without it we'd be more than happy to bring our features back upstream and switch to the original action (added one more since I wrote the comment).

manuelmhtr commented 2 years ago

Hi @fo2rist! Thanks for your reply.

Unfortunately, I think excluding reviewers is a very specific use case, sorry about that. However I'll leave this issue opened to see if more people have the same need and maybe include it in a latter version.

About mixpanel, I think even removing the library there're still the same problems since there's nothing to prevent the library is added again. However, I refactored the telemetry module to make it easier to audit and added a robust test suite, also made it a "premium feature" for sponsors. This way I really have an incentive to ensure the telemetry option works, and is fair for people/companies supporting the project in other way.