-
Notifications
You must be signed in to change notification settings - Fork 135
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
[#299] improvement: Make config type of RSS_STORAGE_TYPE as enum #1052
Conversation
Hi @zuston @jerqi @xianjingfeng I'd like to learn more about your thought on this PR if you are available and any comments and suggestions would be appreciated. I'm sorry for any inconvenience that I may have caused. |
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.
LGTM, thanks @cchung100m
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.
LGTM
@cchung100m Master CI failed. I revert this pr. Could you rebase the latest master and raise a new pr? |
Hi @jerqi Thanks for the notification. I will submit the PR again based on latest master branch. BTW, do we have any ideas that why the Master CI failed? |
Hi @zuston @jerqi @xianjingfeng
Any comments and suggestions would be appreciated if you are available, thank you.
What changes were proposed in this pull request?
improvement: Make config type of
RSS_STORAGE_TYPE
inRssBaseConf
as enumWhy are the changes needed?
Fix: #299
Does this PR introduce any user-facing change?
No.
How was this patch tested?
current UT and integration testing