Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add limit feature #13

Closed
wants to merge 3 commits into from
Closed

Conversation

escudero89
Copy link
Contributor

With this PR, I am trying to add a limit feature that will trim the rows until a certain limit (if defined, otherwise it will work as usual).

Our use-case is a monorepo, in where many people work, and the table grows quite big.

Added test-case and ran the build action (which introduces changes that I did not make but were previously in the master branch).

@manuelmhtr
Copy link
Contributor

Hi @escudero89
Thanks for your contribution! changes are now available in v2.1.0.

@manuelmhtr manuelmhtr closed this Aug 24, 2021
@escudero89 escudero89 deleted the add-limit branch August 24, 2021 09:09
@escudero89
Copy link
Contributor Author

Hi @manuelmhtr, I think I introduced a bug though with the feature 😢 . Could it be that the number of totalReviews are being shorten? I don't quite know why by just reading the code. Or is it working correctly?

Also a nice feature would be to not add the table if it finds that the # Pull reviewers stats text is already in the description (because otherwise it keeps replicating the table, e.g. by opening and closing the PR)

@manuelmhtr
Copy link
Contributor

Hi @escudero89 that's true. I didn't see the bug either, no problem. Since it only happens when the limit option is enabled it won't happen a lot (for now). I'll fix it as soon as possible.

About the table being replicated, indeed that's the intended behavior. However, there was a bug, when the PR description was empty the table is printed up to 2 times. That was fixed before but as you noticed some changes were not included in the build. It should be fixed in v2.1.0 now.

@manuelmhtr
Copy link
Contributor

Hey @escudero89 the bug is fixed now in v2.1.1. Thanks for reporting it!

@escudero89
Copy link
Contributor Author

Gracias Manu :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants