Conversation
Summary of ChangesHello @sehansi-9, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant new feature: an API to provide a detailed historical timeline for any given department. It addresses the complex challenge of tracking departmental changes, ministerial appointments, and historical gaps by intelligently processing relational data. The implementation includes robust data fetching, enrichment, and consolidation mechanisms to present a coherent and accurate historical view, ensuring data integrity and user-friendly output. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new API endpoint to retrieve the historical timeline of a department. The implementation is comprehensive, covering department renaming, ministerial appointments, and handling of appointment gaps. I've provided a few suggestions to improve performance, code structure, and the API contract for better usability. The tests are well-written but could be made more robust against changes in asynchronous execution order. Overall, this is a solid contribution.
…_entities` mocks in organisation service tests.
|
|
||
| # 4. Filter person appointments that overlap with this specific ministry-department period | ||
| enriched = [] | ||
| for ministry_department_relation in all_ministry_department_relations: |
There was a problem hiding this comment.
Can't we run every iteration of this loop in parallel?
There was a problem hiding this comment.
since there are no API calls and the iterations are purely computational in this loop, would it make a difference to use async io? or is there a different approach?
There was a problem hiding this comment.
Since this loop has multiple nested loops in it my understanding is that it would take a lot of time to complete each iteration so since the iterations are independent we can do them in parallel. However, I suppose this depends on the data as well. @ChanukaUOJ what do you think?
This PR introduces a new endpoint to fetch the historical timeline of a department, tracking its movements across different ministries and the ministers appointed during those periods.
closes #36