middlewarehq / middleware

✨ Open-source DORA metrics platform for engineering teams ✨
https://middlewarehq.com
Apache License 2.0
1.15k stars 96 forks source link

Add Functionality for PRs merged without review #207

Open samad-yar-khan opened 6 months ago

samad-yar-khan commented 6 months ago

Why do we need this ?

Acceptance Criteria

Task 1, 2, 3 can be done in seperate PRs.

Further Comments / References

jayantbh commented 4 months ago

If anyone else is interested in this being a part of the app, please comment or react to the thread. :)

RISHIKESHk07 commented 3 months ago

Sir , is this open for work ?

jayantbh commented 3 months ago

Sure! Do remember that because this will be a visual change, you'll need to clear up the visual changes before implementing them.

We already have an idea of how we want to build this, but you're welcome to pitch your idea of how you see this being built. In case you'd prefer to go with our idea itself, that's totally fine.

Let us know how you want to proceed.

RISHIKESHk07 commented 3 months ago

@jayantbh will proceed with your idea and here visual change meant ? , i have not much exp with flask , will work on that and i will have to create a route in pull_requests.py file for fetching the Prs which are not reviewed based on a team_id .

jayantbh commented 3 months ago

By visual change, I mean that if you see the original issue description, you'll see it talks about a UI change as well. That. 🙂

RISHIKESHk07 commented 3 months ago

@jayantbh thanks got it , thought it meant something different

RISHIKESHk07 commented 3 months ago

image @jayantbh the settings here is a form of type validation ? , a bit confused with this part any resource i could use to understand it (get_settings service part and type validation process)

jayantbh commented 3 months ago

Sure, I believe @samad-yar-khan might be able to help here.

samad-yar-khan commented 3 months ago

image @jayantbh the settings here is a form of type validation ? , a bit confused with this part any resource i could use to understand it (get_settings service part and type validation process)

@RISHIKESHk07 get_settings_service() is a getter that fetches the SettingsService after injecting all the dependencies. Here we are first fetching the SettingsService and then fetching a specific setting using get_setting method. The get_setting method fetches a setting for a type and entity. This entity can be a team, user or org as a whole.

RISHIKESHk07 commented 3 months ago

@jayantbh @samad-yar-khan , this is my solution for making a backend route , please correct if i it is flawed (i) create a function merged_not_reviwed in PullRequestAnalyticsService (ii) create function for filtering data , i would find all ids with type "Review " from PullRequestEvent class then use this list to filter out data from the PullRequest class , return the result (iii) create a route ('/teams//prs/not_reviwed_merge') and put the above logic over here .

RISHIKESHk07 commented 3 months ago

@jayantbh does the above look fine ?? \

jayantbh commented 3 months ago

I think it doesn't need to be a separate screen. This could be a small widget thing on one of the existing UIs.

If every metric is on its own page it'll be difficult to get the whole picture from any single point.

RISHIKESHk07 commented 3 months ago

Alright that does sound better than a whole page , do we use a component lib for styling ? , does the backend route creation look fine ?

jayantbh commented 3 months ago

I'll let @samad-yar-khan chime in on the API path.

Visually, it can be shown in the lead time/cycle time sidebar beside the CT/LT breakdown, like this:

image
RISHIKESHk07 commented 3 months ago

@samad-yar-khan Made a Pr

RISHIKESHk07 commented 3 months ago

@jayantbh @samad-yar-khan any suggestions

VipinDevelops commented 3 months ago

Hey @samad-yar-khan , @jayantbh This issue sounds interesting to me, Can I work on the UI part of it ?

jayantbh commented 2 months ago

Hey @VipinDevelops, sure!