Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: mecab - Yet Another Part-of-Speech and Morphological Analyzer https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229927 j.w.r.degoede@xxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@xxxxxxxxxxxxxxxxx |j.w.r.degoede@xxxxxx Flag| |fedora-review? ------- Additional Comments From j.w.r.degoede@xxxxxx 2007-02-26 06:59 EST ------- Some initial Should Fix's: * Better document / explain why you've got an empty base package and then a -base package. I think I get it, mecab-dict will buildrequire mecab-devel (and thus mecab-base) if it would actually require mecab plain, then there would be a problem as mecab plain requires macab-dict, which in turn buildrequires mecab plain, chicken and egg problem. Right? * mecab, not mecab base will go into comps, and thus the %description of mecab is what most end users will see. So the decription of mecab should be what you currently have for mecab-base and then the description of mecab-base would be something like: This package contains the actual mecab-files, the mecab package is just a dummy package things are done this way because ...... * Can't we just avoid all this together by not requiring mecab-dict? I understand that mecab is of no use without dicts, but I think there are multiple dictionaries possbile right? Now if a user installs mecab through yum, he will get the dictionary with the shortest name (as that is what yum will choose). Wouldn't it be better to thus let he user install the dict himself instead of doing this through deps? I think its safe to assume that people who want to use mecab no a bit about it as its very specific, and thus know they should install a dict. * Coding Style, as said above this is all SHOULD not MUST. I notice that you use %{__cmd} everywhere instead of just plain "cmd" the Fedora standard sort of is to use just cmd, so use rm instead of %{__rm} I also find this much easier to read. Notice that this also is what the spec templates used throughout fedora contain. But if you're uses to doing things this way and don't want to change it, thats fine too. Once I'm done reviewing this I'll probably never look at that .spec file again. * I also have my questions about the configfile handling. Does mecab-base not work (as a buildrequire) without a config file? Having one file owned and in 2 different packages is very ugly. Also what will happen if i install multiple dicitonaries? Again wouldn't it be better to just let the user add any dictionaries. Or the best would be I think to have a %post in dict packages where they add themselves to the config file through sed magic, or something like that. This is how its handled for fonts and XFs for example. Related to this I also think that you're getting the noreplace flag wrong, noreplace doesn't meant that it cannot be replaced by another package noreplace (AFAIK) means that if the file is modified and you update to a newer mecab-base, that the upgrade then won't replace the conffile when its modified, IOW AFAIK noreplace is exactly what you want. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review