[Xmltv-devel] tv_augment comments

[Robert Eden]

Hi Geoff,

I *finally* took a look at your tv_augment and have a few comments.

1. We normally don't use a "/beta" directory. I suggest moving to a
filters/tv_augment directory and update Makefile.PL to
configure/install it. Since chances are it will do no harm, I don't
see a problem just making it active in the Makefile.PL. If it could do
harm, the "beta" method we use is commenting it out in Makefile.PL.
(Hmmm don't think CVS can remove directories.. there's a reason to migrate!)

2. As mentioned above, install the tv_augment.pm as
$(INST_LIBDIR)/XMLTV/Augment.pm in the Makefile.PL

3. I think the syntax of the rules.conf is "rule_type_num)
rule_arguments". Is that really the clearest way to do it? What about
"rule_name<space>rule_argument". If you don't want a space separator,
colons are normally used in config files. I like to use a vertical bar
as a separator (I have for years.. there's no EBCDIC vertical bar
character!)

4. In your sample rules file, I would list a few rules of the same
type. As written, it looks like you're numbering the rules.

I don't have much use for the filter, but I know many folks have data
sources that need touching up. It looks like a useful framework.

Robert

[Karl Dietz]

Post by Robert Eden
Hi Geoff,
I *finally* took a look at your tv_augment and have a few comments.

I'm still short on time, so here is just a high level question.

Post by Robert Eden
I don't have much use for the filter, but I know many folks have data
sources that need touching up. It looks like a useful framework.

We already have tv_extractinfo_en and tv_extractinfo_ar. I understand
that the new framework is functionality from the _uk_rt grabber
extracted into a module.

Does that mean that both tv_extractinfo_* postprocessors can be
refactored to make use of the new module where functionality is
duplicated?

Regards,
Karl

[h***@gmail.com]

Post by Karl Dietz
We already have tv_extractinfo_en and tv_extractinfo_ar. I understand
that the new framework is functionality from the _uk_rt grabber
extracted into a module.
Does that mean that both tv_extractinfo_* postprocessors can be
refactored to make use of the new module where functionality is
duplicated?

Hi Karl,

I've not looked at tv_extractinfo_* before but yes there does seem to be some convergence between what they do. tv_extractinfo_* reportedly looks only at description
but the code also parses title in some subs (tv_augment is mostly concerned with title/sub-title/categories).

Some of the routines seem quite 'source' specific though and should really be in the primary grabber (e.g. 'description' starting with "TVM" means a movie, or "(T)" at end of description means teletext subtitles).

I guess it would be possible to write a truly generic rule, e.g. along the lines of "if field xxx matches regex yyy then set field aaa to bbb" to cater for these edge cases?

The rest of the rules could I'm sure be easily incorporated within tv_augment.

Cheers,
Geoff

[Nick Morrott]

Post by Robert Eden
Hi Geoff,
I *finally* took a look at your tv_augment and have a few comments.
1. We normally don't use a "/beta" directory. I suggest moving to a
filters/tv_augment directory and update Makefile.PL to
configure/install it. Since chances are it will do no harm, I don't
see a problem just making it active in the Makefile.PL. If it could do
harm, the "beta" method we use is commenting it out in Makefile.PL.
(Hmmm don't think CVS can remove directories.. there's a reason to migrate!)

By default, git doesn't add empty directories to a repository. If all
files are deleted/moved from a directory in a commit, that directory
will not be visible afterwards.

You can delete directories in CVS, but research suggests you need
shell-level access to the repository. I don't know if SF provide this,
and with a move to git soon it seems moot to worry about it.

Post by Robert Eden
2. As mentioned above, install the tv_augment.pm as
$(INST_LIBDIR)/XMLTV/Augment.pm in the Makefile.PL
3. I think the syntax of the rules.conf is "rule_type_num)
rule_arguments". Is that really the clearest way to do it? What about
"rule_name<space>rule_argument". If you don't want a space separator,
colons are normally used in config files. I like to use a vertical bar
as a separator (I have for years.. there's no EBCDIC vertical bar
character!)

The "num) args" format you're referring to is the numbered list of
comments describing the rules. The vertical bar has always been used
in the actual rules (both uk_rt and Augment.pm).

Post by Robert Eden
4. In your sample rules file, I would list a few rules of the same
type. As written, it looks like you're numbering the rules.
I don't have much use for the filter, but I know many folks have data
sources that need touching up. It looks like a useful framework.

I also noticed the use of 4-space tabs for indentation, which
conflicts with the use of tabs in other core libs, which use 8-space
tabs with mixed indents.

I'll put the indentation analysis into another thread.

Nick