libpagure

#30 Added 4 API methods and their testcases for getting more stats of pagure users
Merged by cverna. Opened by pnemade.
Unknown source master

Download 30.patch
  • user_activity_stats
  • user_activity_stats_by_date
  • list_pull_requests
  • list_prs_actionable_by_user

Signed-off-by: Parag Nemade pnemade@fedoraproject.org

can some please review this pull-request?

looks good to me. @pingou @cqi @sayanchowdhury

:thumbsup:

It would be good to document the return value and its type here.

List?

If username can be None won't that form an invalid request_url above?

Same here - shouldn't username be required and not default to None? Same for date. Won't these form an invalid URL?

It'd be good to document the return value and its type here.

This seems copy/pasted since it is identical to the first method, including the typo. It also seems that this isn't what this method does given the name of the method.

The name would imply that this is not an accurate description of this method.

This description needs to be updated as well.

Similar problems with username here.

It'd be good to document the return value and its type here.

It would be good to document the types of all these parameters, and the parameters in all the other new methods as well.

This description should be updated too.

Similar username problem.

It'd be good to document the return value, its type, and the type of all the parameters.

I recommend tests for these new methods.

rebased onto 29487b9d6a4d5da82c5b3e5d0f5bbf737140546c

Thanks for your brief review @bowlofeggs

I will work on adding tests.

@bowlofeggs Sorry I was away from this work and want to resume here but I also see @cverna started working on writing test framework for this project. So I request please can you merge this? at least let all others use this really important missing functionality from this PR.

Once that PR#34 gets merged I can add tests for these 4 API methods.

@pnemade I just need #34 to be reviewed and +1 then I can merge it. If you want to give it a go please feel free.

yeah I am just looking into it right now ;-)

hm so I am not sure with the merge of all other PR's if this PR still able to merge or not

hm so I am not sure with the merge of all other PR's if this PR still able to merge or not

It looks ok and we can merge it with a merge commit.

if you want you can rebase your branch so that we keep the git history linear. Also do you want to add tests ?

rebased onto 59507ab060c24e097cf69fe36f2fee3967a5f4ee

rebased onto 16c28694477b5b6118069e34e5f96c9588d94bc5

@cverna Can you please review the testcases and if looks good merge this PR?

rebased onto a6e4e7a860b61ce4578816dfef28d919bb773d13

Look good to me. Let's merge.

Thanks

Pull-Request has been merged by cverna

Thank you for your review and merge.
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.