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 include and exclude filters for youtube feed #92

Merged
merged 3 commits into from
Apr 24, 2022
Merged

Add include and exclude filters for youtube feed #92

merged 3 commits into from
Apr 24, 2022

Conversation

romangr
Copy link
Contributor

@romangr romangr commented Apr 24, 2022

Resolves #89

This is my first real Go project so ready for comments.
Also I'm not sure which counter in allStats I should increment in this case so incremented ignored.

@romangr romangr requested a review from umputun as a code owner April 24, 2022 11:08
Copy link
Owner

@umputun umputun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good. A few minor things

boltStore := &store.BoltDB{DB: db}
svc := Service{
Feeds: []FeedInfo{
{ID: "channel1", Name: "name1", Type: ytfeed.FTChannel, Filter: FeedFilter{Include: "Prefix2", Exclude: "title3"}},
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest adding a case with some regex filters. Maybe for channel2 in addition to the current one

@@ -346,6 +363,30 @@ func (s *Service) isNew(entry ytfeed.Entry, fi FeedInfo) (ok bool, err error) {
return true, nil
}

// isRelevant checks if entry matches all filters for the channel feed
func (s *Service) isRelevant(entry ytfeed.Entry, fi FeedInfo) (ok bool, err error) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the name of the function communicating the intent well. This one is not about relevance, but rather filtering, i.e. isFiltered or isAllowed or smth like this

README.md Outdated
@@ -55,10 +55,11 @@ youtube: # youtube configuration, optional
rss_location: ./var/rss # location for generated youtube channel's RSS
channels: # list of youtube channels to download and process
# id: channel or playlist id, name: channel or playlist name, type: "channel" or "playlist",
# lang: language of the channel, keep: override default keep value
# lang: language of the channel, keep: override default keep value
# filter: criteria to include and exclude videos
Copy link
Owner

@umputun umputun Apr 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would add "can be regex" part

@romangr
Copy link
Contributor Author

romangr commented Apr 24, 2022

Pushed fixes for all comments

Copy link
Owner

@umputun umputun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thx!

@umputun umputun merged commit ce34b37 into umputun:master Apr 24, 2022
@romangr romangr deleted the 89-yt-filters branch April 24, 2022 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Include" filter to add only those entries that match regex
2 participants