On 14.03.2010 16:42, Michael Karcher wrote:
No merge of boards_need_enable and the board enable table as I would never get that patch past Luc Verhaegen. But a consistency check for these two tables can be added to the flashrom selftest.
Note, this DEMO patch is broken in that it does not print "-m" parameters anymore.
Signed-off-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Hm. I'm not exactly thrilled about this patch (not your fault). Yes, it reduces a sizable amount of duplication. Yes, it reduces the amount of consistency checking we have to do. Yes, it will probably reduce the number of mismatch bugs each time someone adds a new board. Still, the original reason I didn't want URLs and board names in the same master table is that I personally think URLs are ugly and should not be in flashrom at all. print_wiki.c allowed me to ignore that stuff without having to sacrifice working on any board lists. I can just pretend print_wiki doesn't exist and be happy. ;-)
I won't veto the patch for two reasons: 1. You create most board enables and add most supported boards nowadays, so this is much more "your" turf than mine, and it should be in a state that makes development easier for you. 2. Uwe created most of the printing (and IIRC all of the wiki) code, so he will know if this patch is a good idea or not.
If Uwe acks this, go ahead.
Regards, Carl-Daniel
Am Donnerstag, den 25.03.2010, 21:38 +0100 schrieb Carl-Daniel Hailfinger:
No merge of boards_need_enable and the board enable table as I would never get that patch past Luc Verhaegen. But a consistency check for these two tables can be added to the flashrom selftest.
Hm. I'm not exactly thrilled about this patch (not your fault).
[...]
Still, the original reason I didn't want URLs and board names in the same master table is that I personally think URLs are ugly and should not be in flashrom at all. print_wiki.c allowed me to ignore that stuff without having to sacrifice working on any board lists.
Which is exactly the reason why the tables get out of sync.
We really should have one board database somewhere, and have the board enable table, and the board list (in the Wiki) generated from that. It thus should include (at least) Board vendor/model, testing status (works/broken), type (laptop system/desktop board), optionally DMI/PCI IDs for board recognition, max decode sizes, in future SuperI/O chip, and the board enable function. This table has to be in the flashrom VCS, so a online database is not an option. It will be ugly to have it in source code, I know quite well that Luc does not really like expanding the board enable table for that reason. I see that you don't like expanding the board tables for that reason, but I still think the only way to keep things consistent is to have one master board table, though I have no idea whether this is politically possible and technically handleable. svn merges of patches to "the one big table" will probably be a pain in the <you know what>.
- You create most board enables and add most supported boards nowadays,
so this is much more "your" turf than mine, and it should be in a state that makes development easier for you.
I didn't even realize that writing a board enable involves adding an URL to the wiki table until recently...
- Uwe created most of the printing (and IIRC all of the wiki) code, so
he will know if this patch is a good idea or not.
Uwe asked me to create this demo patch.
Anyway, thanks for your input.
Regards, Michael Karcher
On 25.03.2010 23:25, Michael Karcher wrote:
Am Donnerstag, den 25.03.2010, 21:38 +0100 schrieb Carl-Daniel Hailfinger:
- Uwe created most of the printing (and IIRC all of the wiki) code, so
he will know if this patch is a good idea or not.
Uwe asked me to create this demo patch.
Anyway, thanks for your input.
I'm very sorry if my sort-of-review sounded dismissing. That was unintentional and I wish to apologize for any negative feelings caused by my mail.
If Uwe asked for that patch, I'm sure he will be happy to review and ack. You do have a point with the synchronization problems, and I'll just keep quiet in the future about this area of code.
Regards, Carl-Daniel
Am Donnerstag, den 25.03.2010, 23:59 +0100 schrieb Carl-Daniel Hailfinger:
I'm very sorry if my sort-of-review sounded dismissing. That was unintentional and I wish to apologize for any negative feelings caused by my mail.
No, sorry. Your feedback is very appreciated! This mail does not cause negative feelings, it just shows that the problem of managing the board list in a sane way is complicated and needs discussion!
If Uwe asked for that patch, I'm sure he will be happy to review and ack. You do have a point with the synchronization problems, and I'll just keep quiet in the future about this area of code.
Please continue to input your oppinion. I believe that even negative feedback is better than no feedback - you might still be overruled, though. And this DEMO patch can not be committed, as it kills part of the functionality (as said in the original mail). I just sent it here exactly for the discussion about this approach.
Regards, Michael Karcher
-
Carl-Daniel Hailfinger -
Michael Karcher