Attention is currently required from: Felix Singer, Nico Huber, Richard Hughes, Edward O'Callaghan, Angel Pons, Daniel Campello.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64663 )
Change subject: libflashrom: Allow getting the progress_state from the flashctx ......................................................................
Patch Set 2:
(2 comments)
Patchset:
PS2: I decided to write one more comment in this thread. I feel like we need to try and wrap this thread and focus the code review by the link 64663 on technical questions about the patch and progress feature :) But there are few very important things that I wanted to say, and I will say only the most important, and I am really looking forward to continuing discussing all the rest of the important topics in other places!
First thing first. Angel I appreciate a lot that you spent time and wrote all that to me! I read very carefully and many times. You know, Nico once said to me “you are willing and able to read” and he was right (the exact quote is in gerrit somewhere). I see that we have a lot to talk about, and I am sure we will do (just not on this patch :D)
How should we apologize to you?
You already did. You did it REALLY WELL!
== Two unpleasant situations ==
The best thing about these: both solved as of today! But I know you know this already.
If it makes you feel better, deleting the programmers from Situation 1 had a side effect of deleting their unit tests. Those unit tests were (at that time) the most complicated ones on flashrom, and I was proud of them. It was literally: I did some cleanups/refactorings + I wrote unit tests and the combination of these resulted in discovering a problem which had been hidden for like a year. Yeah, the solution to a problem that was discovered by unit tests was to delete the code together with unit tests. So I sacrificed something that had value to me and I was proud of. But I could understand and agree that resolving the problem is more important. Hopefully this makes you feel better :D
Situation 2 is fixed. It happened way before I joined, but I am very happy that I fixed it. Done and dusted! :)
It went on for about 1.5 years
Sorry if it was not entirely obvious from my message: but it’s not a problem that someone (any one person) decided for whatever reasons, to take a break for 1.5 years. For any number of months/years! It’s fine! Each one of us can decide to take a break, and do it. For me, it is really important to have a team, and to have team members support each other. I can talk on this [very important for me] topic for a long time, but I won’t do it here :) What I wanted to point out, we can’t accuse contributors just because one of the maintainers (of several) took a break. It’s not really a contributor’s problem.
Any one of us can take a break when they need it. And the rest of the people will take care of everything (as best as they can)!
Sounds good! It would be interesting to (maybe?) chat about this in a voice call or similar.
Let’s do it then! Good news is: video call already exists, this is flashrom dev meeting. And, between November and March, the time is more convenient for US folks to join.
I will write one more paragraph about the flashrom dev meeting (hopefully people will forgive me, everyone knows I have special feelings for this meeting :)).
I remember you Angel joined once, the first ever meeting. There were many people joining, and also the agenda was somewhat apprehensive. At the time the first meeting started, the agenda contained 4.5 pages of topics, added by many people (pages -> imagine pages on a printer). Those 4.5 pages contained a lot, from development to politics, and everything in between. We’ve made it through that initial 4.5 pages list of topics around the end of June (we’ve made it wow!!). Also the number of people who join regularly reduced noticeably. So nowadays we are discussing such very exciting topics as: “What if we pack these 3 variables into a ABC struct and pass this struct as the first parameter into a XYZ function?” “Seems like we have two patches on the similar topic , should they be rebased on the top of one another?” “There is a bug which is marked release blocking, what exactly is in scope of this bug?” And so on. TLDR: you are very welcome to join! Between November and March it is late evening EU time.
File include/libflashrom.h:
https://review.coreboot.org/c/flashrom/+/64663/comment/5afd3a01_fac81854 PS2, Line 85: typedef void(flashrom_progress_callback)(struct flashrom_flashctx *flashctx);
Yes about `flashctx`. But with `user_data` I mean the generic (opaque) object passed […] So why not make things simple and pass (progress_state, user_data) ?
I spent some time staring at various code, and I don't know why not. I see that callback is only called internally to flashrom, in a place where flashctx is already available. Get progress state is called externally, but callback is not. So I don't know. It would be great to have patch owners to join here, but I am not sure they will, for $reasons
Let me try once again,
Richard, Daniel, what do you think about passing (progress_state, user_data) into a callback? Would that work for your use case?