redhat-aqe / review-rot

GNU General Public License v3.0
13 stars 16 forks source link

Replace -s -d -n with --age argument #89

Closed mkosiarc closed 5 years ago

mkosiarc commented 5 years ago

Fixes https://github.com/redhat-aqe/review-rot/issues/67, But instead of using --since and --until we decided to use --age.

review-rot --age older 5d 10h outputs MRs/PRs which ale older than 5 days and 10 hours, review-rot --age newer 5d 10h outputs MRs/PRs which are newer than 5 days and 10 hours.

mkosiarc commented 5 years ago

@pbortlov @amarza-rh please review

pbortlov commented 5 years ago

@danrodrig could you please do a review?

amarza-rh commented 5 years ago

I see some refactoring happening here together with the changes.

This makes this review harder than necessary. Can you split these changes into two or more commits? i.e. one with the changes needed and another for the refactoring involved. You can also leave out unnecessary refactoring for a subsequent PR.

Also specify testing done, with expected / actual output if you can.

Thanks!

danrodrig commented 5 years ago

As @amarza-rh commented, would be better to split the PR.

mkosiarc commented 5 years ago

@amarza-rh @danrodrig I removed some unecessary refactoring. The tests relevant for this PR are: