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 Functionality for PRs merged without review #207

Open
3 tasks
samad-yar-khan opened this issue Apr 29, 2024 · 18 comments
Open
3 tasks

Add Functionality for PRs merged without review #207

samad-yar-khan opened this issue Apr 29, 2024 · 18 comments
Labels
enhancement New feature or request good first issue Good for newcomers javascript Pull requests that update Javascript code python Pull requests that update Python code

Comments

@samad-yar-khan
Copy link
Contributor

Why do we need this ?

  • A team may be interested in seeing how many PRs got merged without review in a time duration.
  • This can help in setting processes within the team

Acceptance Criteria

  • Add Python Backend API to fetch Teams PRs merged without review for a team.
  • Update Next JS BFF slices and state to fetch and persist a team PRs merged without review.
  • Add UI component to see PRs merged without review for a team.

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

Further Comments / References

@samad-yar-khan samad-yar-khan added the enhancement New feature or request label Apr 29, 2024
@jayantbh
Copy link
Contributor

jayantbh commented Jul 2, 2024

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

@samad-yar-khan samad-yar-khan added good first issue Good for newcomers labels Jul 3, 2024
@e-for-eshaan e-for-eshaan added javascript Pull requests that update Javascript code python Pull requests that update Python code labels Jul 4, 2024
@RISHIKESHk07
Copy link

Sir , is this open for work ?

@jayantbh
Copy link
Contributor

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
Copy link

@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
Copy link
Contributor

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
Copy link

@jayantbh thanks got it , thought it meant something different

@RISHIKESHk07
Copy link

RISHIKESHk07 commented Jul 19, 2024

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
Copy link
Contributor

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

@samad-yar-khan
Copy link
Contributor Author

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
Copy link

RISHIKESHk07 commented Jul 20, 2024

@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
Copy link

@jayantbh does the above look fine ??
\

@jayantbh
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

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
Copy link

@samad-yar-khan Made a Pr

@RISHIKESHk07
Copy link

@jayantbh @samad-yar-khan any suggestions

@VipinDevelops
Copy link
Contributor

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

@jayantbh
Copy link
Contributor

Hey @VipinDevelops, sure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers javascript Pull requests that update Javascript code python Pull requests that update Python code
Projects
None yet
Development

No branches or pull requests

5 participants