-
Notifications
You must be signed in to change notification settings - Fork 0
Enhance ad config #39
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
Conversation
- Define a new enum AdType to represent different advertisement formats - Includes banner, native, video, and interstitial ad types
- Removed the `localAdsCatalog` map from the `remoteConfigsFixturesData` list - This change simplifies the fixture data by eliminating redundant local ad information
- Remove LocalAd import - Remove localAdsCatalog property from AdConfig class - Update props list in AdConfig class - Update copyWith method in AdConfig class
- Replace wildcard import with specific imports for enums and models - Add imports for ArticleInterstitialAdConfiguration and InArticleAdSlotConfiguration - Update import path for enums
- Remove LocalAd class and its generated JSON serialization file - This model was likely intended for serving local advertisements but is no longer needed
- Removed Ad class and its generated JSON serialization file - Deleted related test files for Ad and LocalAd - This change simplifies the codebase by removing unused advertisement-related code
- Remove `local_ad.dart` export from config.dart - Add `local_ad.dart` export to feed_decorators.dart - Update ad.dart by removing itself as an export
- Create LocalAd class to define a single custom ad for local serving - Include properties for id, title, subtitle, imageUrl, targetUrl, and adType - Implement.fromJson and toJson methods for JSON serialization - Add copyWith method for creating modified copies of LocalAd instances
- Remove 'ad' case label - Add 'localAd' case label - Update JSON parsing to use LocalAd model
- Rename `Ad` to `LocalAd` in the list of feed item subtypes - Update documentation to reflect the correct model name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @fulleni, 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 significantly refactors the ad configuration and models within the 'core' package. The primary goal is to streamline how ads are defined and integrated into the application's feed system. By removing the generic 'Ad' model and enhancing 'LocalAd' to be a direct 'FeedItem', the changes improve consistency and maintainability of ad-related data structures.
Highlights
- Refactored Ad Models: The generic 'Ad' model has been removed, and the 'LocalAd' model has been promoted to a 'FeedItem' and relocated to the 'feed_decorators' directory, streamlining ad representation.
- Centralized AdType Enum: The 'AdType' enum is now defined in its own dedicated file within the 'enums' directory, enhancing its reusability and accessibility across the codebase.
- Streamlined AdConfig Structure: The 'AdConfig' model no longer directly manages a 'localAdsCatalog', simplifying its responsibilities and reducing coupling.
- Updated FeedItem Deserialization: The 'FeedItem' factory constructor has been updated to correctly deserialize 'LocalAd' instances, ensuring proper handling of local ad content within the application's feed.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request provides a solid refactoring of the ad configuration and models. Key changes include replacing the Ad
model with an enhanced LocalAd
that now acts as a FeedItem
, removing the localAdsCatalog
from AdConfig
to improve decoupling, and extracting the AdType
enum for better organization. These architectural improvements make the system more modular and maintainable. The changes are well-implemented across the codebase, including updates to tests and documentation. I have one minor suggestion to improve the clarity of a test case.
Status
READY
Description
This pull request significantly refactors the ad configuration and models within the 'core' package. The primary goal is to streamline how ads are defined and integrated into the application's feed system. By removing the generic 'Ad' model and enhancing 'LocalAd' to be a direct 'FeedItem', the changes improve consistency and maintainability of ad-related data structures.
Type of Change