#31: Do proper checking for flash erase for SST FWH parts ---------------------------+------------------------------------------------ Reporter: stepan | Owner: stepan Type: defect | Status: new Priority: major | Milestone: Enhance the flashrom utility Component: flashrom | Version: v2 Keywords: | Include_gantt: 0 Dependencies: | Due_assign: MM/DD/YYYY Due_close: MM/DD/YYYY | ---------------------------+------------------------------------------------ Instead of checking the first byte only, the whole part is checked now. This will detect any improper erase. The patch removes a FIXME. :-)
Signed-off-by: Stefan Reinauer stepan@coresystems.de
#31: Do proper checking for flash erase for SST FWH parts ----------------------------+----------------------------------------------- Reporter: stepan | Owner: stepan Type: defect | Status: assigned Priority: minor | Milestone: Enhance the flashrom utility Component: flashrom | Version: v2 Resolution: | Keywords: Include_gantt: 0 | Dependencies: Due_assign: MM/DD/YYYY | Due_close: MM/DD/YYYY ----------------------------+----------------------------------------------- Changes (by stepan):
* priority: major => minor * status: new => assigned
Instead of checking the first byte only, the whole part is checked now. This will detect any improper erase. The patch removes a FIXME. :-)
... and adds one, heh :-)
Signed-off-by: Stefan Reinauer stepan@coresystems.de
The patch never made it to the mailing list. To review the patch I now have to:
1) Click the link in the trac mail; 2) Find the correct patch in the list, click it; 3) Scroll all the way to the bottom, click "download original format"; 4) Remember the file name (and see that it has a MIME type of; "text/x-diff", whatever that is); 5) Open the file in some editor; 6) Copy all text; 7) Press "reply" somewhere in the trac page; 8) Paste that patch; 9) Fixup all mangling (insofar as possible) that the HTML textfield did to the whitespace etc.; 10) Type my comments; 11) Click "submit"; 12) See my mail on the mailinglist and discover it doesn't do mail threading; 13) Clean up the garbage file.
While with reviewing a patch on a mailing list I do
1) Hit the "reply to all" button (so that all interested people will see my comments first thing!); 2) Type my comments.
This isn't working too well, even for just *reading* submitted patches I'll have to do steps 1) and 2) now, and close the browser window and return to the mail window; this is a non-linear workflow, while with mail I just go scroll-scroll-scroll-scroll.
LinuxBIOS is really low-volume but it already almost takes me longer to keep up with the patch flow than it takes for Linux.
Don't take this as a flame, I just need a working working environment :-)
Segher
Segher Boessenkool wrote:
LinuxBIOS is really low-volume but it already almost takes me longer to keep up with the patch flow than it takes for Linux.
Don't take this as a flame, I just need a working working environment :-)
OLPC uses trac as well and I'm going to have to side with Segher.
Although it does keep things together its a much larger PITA to work with. And I find it cumbersome to find the stuff I'm looking for unless I just happen to remember the Trac ID#.
However, An issue tracker is essential. I don't want us to stop using it but the workflow could be a lot nicer.
Finding/implementing some sort of trac<->mailing list gateway would be ideal.
This is really OT for this trac ticket so I'm replying to the list rather than in trac.
* Richard Smith smithbone@gmail.com [061105 21:54]:
Although it does keep things together its a much larger PITA to work with. And I find it cumbersome to find the stuff I'm looking for unless I just happen to remember the Trac ID#.
The search function is not worse than searching the mailing list..
Finding/implementing some sort of trac<->mailing list gateway would be ideal.
Ok. So what should the gateway look like?
I am pretty unreligious about which system to use, but we should stay with what we decided to use at least until we find a better solution or we just fix the solution we have. But this is only possible if there's a common sense about the requirements.
As always: He who sends a working patch usually wins.
Stefan Reinauer wrote:
The search function is not worse than searching the mailing list..
Its on organizational thing rather than a brute force search function thing.
Finding/implementing some sort of trac<->mailing list gateway would be ideal.
Ok. So what should the gateway look like?
I am pretty unreligious about which system to use, but we should stay with what we decided to use at least until we find a better solution or we just fix the solution we have. But this is only possible if there's a common sense about the requirements.
As always: He who sends a working patch usually wins.
I don't claim to have solutions... It's a complex task. I'll continue to use trac until we get something worked out.
I'm just backing up Segher that dealing with patch review in Trac is not as easy as looking at them on the mailing list.
One of the OLPC guys is a trac developer and maintainer of our trac interface. I'll try to buy him a beer when I'm up there next week and see what his thoughts are on something like this.
Although it does keep things together its a much larger PITA to work with. And I find it cumbersome to find the stuff I'm looking for unless I just happen to remember the Trac ID#.
The search function is not worse than searching the mailing list..
Sure it is. I have my mail archive, nicely sorted and tagged, exactly as _I_ want it and I can easily drag out any mail I want. Having to use a separate application (and web-based too of all things) to access a fraction of the information universe I'm dealing with doesn't scale too well.
Finding/implementing some sort of trac<->mailing list gateway would be ideal.
Ok. So what should the gateway look like?
I am pretty unreligious about which system to use, but we should stay with what we decided to use at least until we find a better solution or we just fix the solution we have. But this is only possible if there's a common sense about the requirements.
Well here's some of my thoughts to kick this off then, please discuss:
1) All developers should be aware of all patches; they can just press delete if they don't like the subject ==> All patches should be sent to the mailing list.
2) It should be as easy as possible to discuss patches, in free form, mixing patch snippets with prose (and flames and general silliness). When a patch author thinks he's had enough comments, he sends a new patch with some fixes and/or changes and perhaps an Acked-by: line. ==> All patches should be discussed on the mailing list; patches should preferably be sent inline (or if you really can only send attachments without getting mangled or word-wrapped patches, at least make sure the attachments are type text/plain and hopefully not encoded with binhex or quoted-unreadable or whatever).
3) No patches should go into a SCM tree without agreement from the tree's maintainer. ==> When a maintainer thinks a patch is good enough (after waiting a bit for discussion, perhaps), he signs off on the patch and either commits it or tells the author it's okay to commit. People read the SCM reflector to know when things went in (there are smallish delays often), and also to see what patches went in that they themselves didn't read (or participate in) (all of) the discussion for, but might be interesting for them nevertheless ["reading the changelog"]; when people are caught checking in stuff unauthorised (and they _will_ be caught) we can scramble their ssh key or force them to work from dialup or whatnot.
3) There is no need to keep all proposed patches in a tracker of any kind: software archeologists can go dig in the mailing list archives; for everyone else, all that matters is _what did actually change_ in the master repository and _why_, and the actual patches and check-in comments in the SCM tell that story. For people with short-term memory loss, there is this wonderful invention called the "threading mailer". ==> A tracker is a very useful tool for combined SCM monitoring, problem report handling, keeping track of the road map, etc.; it is *not* a good tool to organise people's personal code development (I prefer git and quilt, thanks), and certainly not a good tool for cooperative development. We need to work _closer_ together, in a sometimes non-linear or slightly ad-hoc fashion; communicate, and communicate freely; we don't need to be forced to sort and organise every little piece of work we do into millions of little boxes. Use the tracker for what it's good for: keeping track of the end result (the SCM, the problem database, the "big picture" of the current state of affairs). We don't need some unrelated tool to ordain how we should conduct or day-to-day business processes.
4) For accountability or just a paper trail, we have the Signed-off-by: lines, and those should stay with the patches at all time or they become meaningless. ==> The tracker can _track_ the sign-offs, that's fine; but it shouldn't be the main repository of-em (the SCM is for merged patches; all patches proposed to be applied but still in flight carry them inline with the patch).
I hope this wasn't too long :-)
As always: He who sends a working patch usually wins.
Nah, whoever gets the Signed-off-by: stamp-of-approval on his patch wins :-)
Segher
* Segher Boessenkool segher@kernel.crashing.org [061105 21:40]:
The patch never made it to the mailing list. To review the patch I now have to:
I contacted the trac folks for that problem. Attachments should make it into notification mails.
Don't take this as a flame, I just need a working working environment :-)
Of course not. ;-) There was public discussion on the symposium about which tracker to use, and we all participated, so I am of course glad about your late feedback.
Nobody wanted bugzilla nor roundup. Nobody opposed to trac either, so we worked around some problems, and bought us into some others.
Patch management on the mailing list just does not work. It never will.
Here's the golden open source rule that applies to trac as well: If you want it fixed, send a fix ;-)
Stefan
Stefan Reinauer wrote:
Patch management on the mailing list just does not work. It never will.
The Linux Kernel guys seem to think otherwise.
* Richard Smith smithbone@gmail.com [061105 22:03]:
Stefan Reinauer wrote:
Patch management on the mailing list just does not work. It never will.
The Linux Kernel guys seem to think otherwise.
1. There are _lots_ of people hired full time to check no patches get lost. 2. Patches still get lost or stay ignored pretty often.
The Linux Kernel guys seem to think otherwise.
- There are _lots_ of people hired full time to check no patches
get lost.
They're not lost, they are still in people's inboxes (unless they deleted them) and in the ML archive.
There is only one guy working fulltime taking care of all orphan Linux patches (well not fulltime really, even), FWIW.
- Patches still get lost or stay ignored pretty often.
If the author still cares, he can ping the patches after some certain period of time (don't be too impatient).
If really everyone is too busy or just no one cares, well, tough luck, that has nothing to do with what tools they use to run their business.
Of course it helps for a huge project like Linux that someone like MM regularly sweeps up all (small) patches that look good enough to him and/or whoever reviewed them on the list, to have those patches simmer in his experimental tree for a while.
At the other side of the spectrum, there are projects like GCC, where patches frequently are ignored or at least not reviewed for a month or more. Most such patches are huge invasive patches requiring in-depth specialistic knowledge of what that patch is touching.
Does that mean such patches should then just be "let in" without review?
Does it mean that we should keep a subset (namely, the new patch contents) of our development mailing list in a separate database (the tracker)? Would that really be harder to ignore (for the "bad guys") or easier to keep up with (for the "good guys")?
Segher
The patch never made it to the mailing list. To review the patch I now have to:
I contacted the trac folks for that problem. Attachments should make it into notification mails.
Thanks!
Don't take this as a flame, I just need a working working environment :-)
Of course not. ;-) There was public discussion on the symposium about which tracker to use, and we all participated, so I am of course glad about your late feedback.
Nobody wanted bugzilla nor roundup.
I wanted (a heavily patched) Bugzilla, but I of course know all the reasons people hate it. It "works for me" though ;-)
Nobody opposed to trac either, so we worked around some problems, and bought us into some others.
Yeah, it seems trac isn't too good a fit for us yet. Some patching will help, I have no idea how much effort it is. Do we have contact with some friendly trac developer(s) who could help us out?
Patch management on the mailing list just does not work. It never will.
It works fine (and has worked fine since basically forever) for: -- linux-kernel and all its children lists; -- gcc-patches; -- binutils; -- libc-alpha; -- many other GNU mailing lists; -- and who knows what else.
The only thing that's needed are some "maintainers" (or whatever you want to call them, "gatekeepers" or whatever) who review patches and ACK or NAK them for inclusion into the main tree. And hey, we have that already.
Even if patch review is done on the mailing list a) all actual checkins still go over SVN so trac sees them; and b) there's nothing that stops people from associating a mailing list mail with a trac ticket (either manually write a comment saying "patch posted at this-or-that-ml-archive-address", or have trac snoop the list and recognise certain tags and attach the stuff by itself (for example, "Closes: #21" and "Patch-for: #21", etc.)
Here's the golden open source rule that applies to trac as well: If you want it fixed, send a fix ;-)
Trust me I would if I could. But I a) don't have the time; and b) I don't have the configuration files or the exact source code for our installed version of trac.
Segher
On Sun, Nov 05, 2006 at 09:40:04PM +0100, Segher Boessenkool wrote:
The patch never made it to the mailing list. To review the patch I now have to:
- Click the link in the trac mail;
The patches should really be sent together with the notifications. This will make your list a lot shorter. This will be basically almost equal to a mail-only handling...
Optionally, you could use the web only.Example:
1) Go to http://tracker.linuxbios.org/trac/LinuxBIOS/ticket/26 2) Click on a patch. It'll be displayed syntax-highlighted in a nice format in your browser (or else your browser sucks ;)
The problem is, you currently cannot "reply" to a patch there (as is possible with Trac comments) and insert your comments. Would this be nice-to-have? In that case we could fix Trac to do just that.
The nice thing about Trac is that it is very customizable. We can fix it with patches, configure it, add plugins, whatever. We just need to have a common understanding of what features we expect from it, the rest is "easy"...
- Find the correct patch in the list, click it;
- Scroll all the way to the bottom, click "download original format";
- Remember the file name (and see that it has a MIME type of; "text/x-diff", whatever that is);
- Open the file in some editor;
- Copy all text;
- Press "reply" somewhere in the trac page;
- Paste that patch;
- Fixup all mangling (insofar as possible) that the HTML textfield did to the whitespace etc.;
Enclose patches you want to comment on with
{{{ foo }}}
That will prevent lots of mangling.
- See my mail on the mailinglist and discover it doesn't do mail threading;
I think it does a "flat" threading, i.e. all comments are marked as replies to the original issue. It could be better, but at least the mails are held together in a thread at all :)
While with reviewing a patch on a mailing list I do
- Hit the "reply to all" button (so that all interested people will see my comments first thing!);
Well, that's a matter of taste. I have to delete whole _threads_ of duplicated mails all the time, just because I replied to a single mail in that thread. WTF? I'm subscribed, I _already_ get all the emails, why should I be CC'd explicitly all the time?
That's why I remove all the CC's from every mail I reply to. I only reply to the list. End of rant ;-)
Uwe.
On Sun, Nov 05, 2006 at 11:39:03PM +0100, Uwe Hermann wrote:
Well, that's a matter of taste. I have to delete whole _threads_ of duplicated mails all the time, just because I replied to a single mail in that thread. WTF? I'm subscribed, I _already_ get all the emails, why should I be CC'd explicitly all the time?
You can configure Mailman to not send you another copy if it sees your subscribed address among the recipients. See your options page for the list.
//Peter
On Mon, Nov 06, 2006 at 12:22:41AM +0100, Peter Stuge wrote:
On Sun, Nov 05, 2006 at 11:39:03PM +0100, Uwe Hermann wrote:
Well, that's a matter of taste. I have to delete whole _threads_ of duplicated mails all the time, just because I replied to a single mail in that thread. WTF? I'm subscribed, I _already_ get all the emails, why should I be CC'd explicitly all the time?
You can configure Mailman to not send you another copy if it sees your subscribed address among the recipients.
Aah, much better now :) Thanks, I didn't know about that option.
Uwe.
#31: Do proper checking for flash erase for SST FWH parts -----------------------------------+---------------------------------------- Reporter: stepan | Owner: stepan Type: defect | Status: assigned Priority: minor | Milestone: Enhance the flashrom utility Component: flashrom | Version: v2 Resolution: | Keywords: Due_close: MM/DD/YYYY | Include_gantt: 0 Dependencies: | Due_assign: MM/DD/YYYY Patchstatus: patch needs review | -----------------------------------+---------------------------------------- Changes (by uwe):
* patchstatus: => patch needs review
#31: Do proper checking for flash erase for SST FWH parts -----------------------------------+---------------------------------------- Reporter: stepan | Owner: stepan Type: defect | Status: assigned Priority: minor | Milestone: Enhance the flashrom utility Component: flashrom | Version: v2 Resolution: | Keywords: Due_close: MM/DD/YYYY | Include_gantt: 0 Dependencies: | Due_assign: MM/DD/YYYY Patchstatus: patch needs review | -----------------------------------+---------------------------------------- Comment (by segher):
Every patch needs review, unless it already got it, and then it's either back to the drawing table, or that patch get's checked in :-)
If total_size is indeed the size of flash that got erased, it looks good to me. But Stefan himself knows best, he should just go ahead and apply the patch if no one complains soon (like, wait for tomorrow morning or something like that).
Segher
#31: Do proper checking for flash erase for SST FWH parts -----------------------------------+---------------------------------------- Reporter: stepan | Owner: stepan Type: defect | Status: assigned Priority: minor | Milestone: Enhance the flashrom utility Component: flashrom | Version: v2 Resolution: | Keywords: Due_close: MM/DD/YYYY | Include_gantt: 0 Dependencies: | Due_assign: MM/DD/YYYY Patchstatus: patch needs review | -----------------------------------+---------------------------------------- Comment (by uwe):
Replying to [comment:3 segher]:
Every patch needs review, unless it already got it, and then it's either back to the drawing table, or that patch get's checked in :-)
Sure, but we need a state which tells us that there is a patch at all (not all tickets have a patch), and in which state it is (has it already been reviewed? does it need more work? is it ready to be comitted?) etc.
This is mostly used for the shiny new "Patch Queue" I created yesterday: http://tracker.linuxbios.org/trac/LinuxBIOS/report/9 (or click View Tickets -> Patch Queue)
That should make it easier to track which tickets have patches and in which state. Reviewers or people who want to commit patches which have been approved can simply work on the patch queue and make it smaller.
If total_size is indeed the size of flash that got erased, it looks good to me. But Stefan himself knows best, he should just go ahead and apply the patch if no one complains soon (like, wait for tomorrow morning or something like that).
Add an Acked-by: if it looks good to you. One important part of the sign- off procedure is that we have sort of a group-review, i.e. at least one other developer has to review the patch and give it his/her ACK (the other important part is copyright and authorship tracking, of course).
#31: Do proper checking for flash erase for SST FWH parts -----------------------------------------------+---------------------------- Reporter: stepan | Owner: stepan Type: defect | Status: assigned Priority: minor | Milestone: Enhance the flashrom utility Component: flashrom | Version: v2 Resolution: | Keywords: Due_close: MM/DD/YYYY | Include_gantt: 0 Dependencies: | Due_assign: MM/DD/YYYY Patchstatus: patch is ready to be committed | -----------------------------------------------+---------------------------- Changes (by uwe):
* patchstatus: patch needs review => patch is ready to be committed
Comment:
Acked-by: Uwe Hermann uwe@hermann-uwe.de
Untested, but the patch looks good to me.
#31: Do proper checking for flash erase for SST FWH parts -------------------------+-------------------------------------------------- Reporter: stepan | Owner: stepan Type: defect | Status: closed Priority: minor | Milestone: Enhance the flashrom utility Component: flashrom | Version: v2 Resolution: fixed | Keywords: Dependencies: | Patchstatus: patch has been committed -------------------------+-------------------------------------------------- Changes (by uwe):
* patchstatus: patch is ready to be committed => patch has been committed
-
LinuxBIOS -
Peter Stuge -
Richard Smith -
Segher Boessenkool -
Stefan Reinauer -
Uwe Hermann