CodelyTV / pr-size-labeler

🏷 Visualize and optionally limit the size of your Pull Requests
https://github.com/marketplace/actions/pull-request-size-labeler
MIT License
336 stars 58 forks source link

feature: Add support for ignoring files (#9) #33

Closed favmd closed 2 years ago

favmd commented 2 years ago

The code basically does what has been described / discussed in issue #9:

Plus: Minor modifications:

Note however that my code does have its LIMITATIONS and should be tested by someone else too:

I can live with that limitation due to the fact that we don't open PRs with hundreds of changed files, but this is the reason why my code will fall back to the old code if files_to_ignore is empty: to ensure that someone not using the new parameter does not have this limitation at all and falls back to the API endpoint without /files at the end.

rgomezcasas commented 2 years ago

Amazing! I'll try it today.

What do you think about changing the files_to_ignore value to be comma-separated instead of whitespace so we can support filenames with whitespaces?

About the other limitations:

What do you think? 🙂

favmd commented 2 years ago

Amazing! I'll try it today.

Nice! That was fast! 👍

What do you think about changing the files_to_ignore value to be comma-separated instead of whitespace so we can support filenames with whitespaces?

About the other limitations:

  • Global files_to_ignore: I would merge this as-is, and open an issue to support glob patterns

  • Only the first 100 files are taken into account: I would add this limitation to the readme and open an Issue

What do you think? 🙂

@rgomezcasas You're totally right: The limitation that only the first 100 files are taken into account - when files_to_ignore is set - should definitely be mentioned prominently in the README.

Regarding the other limitations: I would like to suggest to merge this PR as-is and move all ideas into new ✨ feature issues, so that there is

All of those changes could be implemented to be backward compatible.

Regarding the split character (will probably be implemented by setting the IFS in bash!?): Let's just make it configurable! Some people seem to prefer whitespaces (me), others want comma-separated (you), I can easily imagine that there's more out there, so I suggest a new issue with a new argument:

I would not try to quickfix this PR (too often those quickfixes add subtle bugs).

Let's go for new issues instead.

Sounds good?

rgomezcasas commented 2 years ago

Agree! Doing some tests in here before merging to main 😊