uilianries
commented
3 years ago 3427 Still waiting for improvements.
3877 LGTM
3962 LGTM
4032 LGTM
mental note: Track my blocked review, otherwise I can delay more PRs.
Open prince-chrismc opened 3 years ago
uilianries
commented
3 years ago mental note: Track my blocked review, otherwise I can delay more PRs.
SSE4
commented
3 years ago @prince-chrismc may you add some legend with icons meaning?
prince-chrismc
commented
3 years ago may you add some legend with icons meaning?
That sounds like an excellent idea! I'll see if I can squeeze it in today/tomorrow 😄
prince-chrismc
commented
3 years ago @SSE4 I updated the code to add a legend. Let me know if it helps
mathbunnyru
commented
3 years ago @prince-chrismc small note: Age: 76d11h16m40.084126774s
What does it mean and it doesn't look pretty =)
prince-chrismc
commented
3 years ago It's Golangs internal date formatting 👾 Is the average age of the pull requests... there's a lot of pull requests bloating the number
mathbunnyru
commented
3 years ago It's Golangs internal date formatting 👾 Is the average age of the pull requests... there's a lot of pull requests bloating the number
Thanks for the information. If you have some spare time, I would be glad if you changed formatting and the wording to smth like Average PR age.
prince-chrismc
commented
3 years ago Done!
ericLemanissier
commented
3 years ago Is it me, or was the PR https://github.com/conan-io/conan-center-index/pull/5440 never mentioned in this post ? EDIT: it's expected, because all the reviews happened in less than an hour
prince-chrismc
commented
3 years ago Someone was really busy...
dvirtz
commented
3 years ago prince-chrismc
commented
3 years ago 5605 has no approval, 5606 is labeled, 5914 no approvals
I will one day expand this to include PRs like yours but we have so many open as it is... I assume if someone starts reviewing they see it through but you just convinced me that's not always true...
🤔
redradist
commented
3 years ago Can somebody review this MR Initial support for wasmtime package ?
prince-chrismc
commented
3 years ago Please be patient! https://github.com/conan-io/conan-center-index/pull/6083#issuecomment-873398554 just passed 11 minutes ago, and it's on the board... If you look at time in review graph it's very normal for it to take ~7 days.
There's a lot of PRs and we try our best to get to them all
SSE4
commented
3 years ago @prince-chrismc I see that builds with running (waiting for CI) status are added into the table. is it expected?
prince-chrismc
commented
3 years ago I see that builds with running (waiting for CI) status are added into the table. is it expected?
Yes! I am optimistic they will pass. It's the "ready to merge" where I note a warning
madebr
commented
3 years ago @prince-chrismc I have an idea for a graph, but am uncertain whether it will work. On the x-axis: datetime, y-axis: days open The graph would be a boxplot with (colored) lines of 10, 20, ... , 90, 100% of pr's were open this long. I think very old pr's (such as my cpython pr) are skewing your plots.
Is there an easy way to get the raw data? What I need is, for each pr: datetime opened, datetime closed. To filter further, the results of c3i would also be useful: result + datetime.
It would be nice if your github actions would publish this data as an artifact.
prince-chrismc
commented
3 years ago I haven't read your whole comment but there's tons of data we can extra... the visual is limited to OSS graph generators
It would be nice if your github actions would publish this data as an artifact.
It's all pushed to a second branch called [raw-data](https://github.com/prince-chrismc/conan-center-index-pending-review/tree/raw-data)
prince-chrismc
commented
3 years ago I think very old pr's (such as my cpython pr) are skewing your plots.
I 100% agree, I have cut them out of the graph by setting a ceiling... they do not kill the moving average so I still include them and I have limited the start page so not to go so far back... you can see me playing with values here
Since I have a 30 min break, let's see if I understood this...
On the x-axis: datetime, y-axis: days open The graph would be a boxplot with (colored) lines of 10, 20, ... , 90, 100% of pr's were open this long.
Stacked is supported by current library
I think it would be more clean then the thin line on the closed date (I did originally use the open date but it did not allow to spot trends)
madebr
commented
3 years ago The graph is more or less what I meant. I propose a color coding from green over orange to red. It feels more "naturel".
I would set y-range to a value that results in a pretty graph. :smile: For the x-range, perhaps we should have a general graph (=several months) and a detailed graph (=last month), such that we can compare how we're doing now compared to X months ago.
I had a look at the raw data, but found it already too summarized. I was hoping to have more details for each pr because the number of days a pr is open is not always useful. It's useful to know whether it was merged in the end, closed by the author, closed by stale-bot, rejected by us, ... Also knowing when c3i started to successfully build a pr is useful, because an old pr that fails to build also skews the data.
prince-chrismc
commented
3 years ago I propose a color coding from green over orange to red. It feels more "natural".
Something like green is the fastest 50% of PRs for that day, orange for 75%, and red is all of them?
I would set y-range to a value that results in a pretty graph.
This is really freaking hard! The library only supports equally spaced values and the majority are tiny numbers but the rest are huge... it's hard to do
For the x-range, perhaps we should have a general graph (=several months) and a detailed graph (=last month), such that we can compare how we're doing now compared to X months ago.
I really love this idea! it's already a task I have written in #11
I had a look at the raw data, but found it already too summarized. I was hoping to have more details for each pr because the number of days a pr is open is not always useful. It's useful to know whether it was merged in the end, closed by the author, closed by stale-bot, rejected by us
This bot only exclusively looks at those that were merged... so nothing else is present (i did not find them valuable).
I'd love to expand the bot to include more information... per day counts are hard since they need time to accumulate... how ever inspecting comments each PR is doable. example
↪️ If you have ideas or suggestions, I'd love to add them.
Also knowing when c3i started to successfully build a pr is useful, because an old pr that fails to build also skews the data.
Far too often I see us helping the author getting it to pass (there tends to be reviews/comments before it passes and I am not sure how well that would work with the proliferant "Unexpected Error" plague 😷) ... on some occasions we let them drag or put them on hold...
I did consider looking at the "time in review" as the gap from the first to last... how ever I realized sometimes we put comments and not reviews and it made some of the PRs reflect inaccurate very short durations... however 6 months later I am very open to re-opening the topic
prince-chrismc
commented
3 years ago I am shocked we are merging more PRs then closing compared to the past
@madebr How's this? #21
madebr
commented
3 years ago Honestly, the graph is not telling me much. Apart from that, it's nice to see that prs are getting merged instead of being wasted/closed. On such long timescales, some averaging is required or data should be grouped per week/month.
If you really want to work further on this, I can only suggest to extract all data and save it to a csv. Perhaps, by playing with the data using pivot tables, trends can become clearer and such graph can be converted into go code.
prince-chrismc
commented
3 years ago Okay, taking some off so I finally have a chance to play with this... I am not sure I'll get exactly what you were hoping for but I really love the idea. (sorry I dont understand your vision)
🚀 There are weeks we closed every PR 😱
madebr
commented
3 years ago Nice graph! It immediately tells me that ~10% of all prs get closed without merging. Also, there are few old pr's that just don't get closed and we are unable to get merged.
Using more shades of purple/red, we could how long it took until a pr was closed/merged.
:rocket: There are weeks we closed every PR :scream:
That must have been a productive week! :rofl: What week was it? Perhaps it was a week with only one pr?
Weeks with little activity perhaps don't deserve equal visibility. I currently don't have an idea to account for this though.
prince-chrismc
commented
3 years ago Nice graph!
Thank you! 🤗
Weeks with little activity perhaps don't deserve equal visibility.
:100: I was thinking, there's a day with the most opened PRs (it's mid 30s) I can probably pad all of them to that height with alpha... This was weeks with low activity have smaller bars... however I like the color ordering being consistent (it's satisfying to look at).
prince-chrismc
commented
3 years ago 
prince-chrismc
commented
3 years ago ➡️ I can use feedback on the colors
madebr
commented
3 years ago I think the colors look great! What do the 2 purple shades mean? I miss a legend 😄 The x and y labels are also not readable.
I like the simplicity of the graph. Btw, it looks like this graph is not very cci specific. So you can re-use it for other projects.
prince-chrismc
commented
3 years ago What do the 2 purple shades mean? I miss a legend
Forgot to write it xD dark purple === less then 7 days to merge
The x and y labels are also not readable.
They do not exist, because of how the graph is made they are not supported I need to play with it a little more see it I can add some text 🤞
it looks like this graph is not very cci specific. So you can re-use it for other projects.
That is absolutely correct, short of the hardcoded API calls but those can easily be changes
madebr
commented
3 years ago Would the graph above, but only applied to:
be useful? These are the 3 main types of prs created. Prs bumping versions should have a shorter "age" then prs adding new recipes.
prince-chrismc
commented
3 years ago One graph for each? Might be interesting to see if adds and modifications have the same trends 🤔
If you meant limit the included PRs theres very few, I played with it on the other graph, wasn't worth while
madebr
commented
3 years ago I have another crazy idea. Create a gif/video. You have probably seen those animated videos on https://www.reddit.com/r/dataisbeautiful/. Would it be nice to see grahs like in https://github.com/prince-chrismc/conan-center-index-pending-review/issues/1#issuecomment-903336157, but animated in time?
prince-chrismc
commented
3 years ago I have another crazy idea. Create a gif.
I had the same reaction 🤣 It would be amazing. GIFs are natively support in golang so it should be doable. http://tech.nitoyon.com/en/blog/2016/01/07/go-animated-gif-gen/
Lemiort
commented
3 years ago Um, what? Why my PR is not in the list to review? https://github.com/conan-io/conan-center-index/pull/6944
prince-chrismc
commented
3 years ago Um, what? Why my PR is not in the list to review? conan-io/conan-center-index#6944
Someone reviewed it already and it was not approved
If you have ideas how to catch PRs like that (where someone slacked off on a follow up) I'd be glad to refine my bot :)
madebr
commented
3 years ago @prince-chrismc Why doesn't conan-io/conan-center-index#7190 appear in the list?
prince-chrismc
commented
3 years ago no approval and... it has a review from redradist (which the bot assume you need to make changes) after your last commit.
madebr
commented
3 years ago Ah, thanks for explaining the bot's behavior.
madebr
commented
3 years ago @prince-chrismc This analysis of the cpython git history might interest you: https://lukasz.langa.pl/f15a8851-af26-4e94-a4b1-c146c57c9d20/
MartinDelille
commented
2 years ago My PR doesn't appear here: https://github.com/conan-io/conan-center-index/pull/8554
Any idea?
ericLemanissier
commented
2 years ago maybe because of the Unexpected Error label ? you can probably restart the build to remove it
prince-chrismc
commented
2 years ago maybe because of the
Unexpected Errorlabel ? you can probably restart the build to remove it
That was certainly why :+1:
prince-chrismc
commented
2 years ago I need to make graphs just for @SpaceIm 💟
Almost beat your last record
prince-chrismc
commented
2 years ago I have another crazy idea. Create a gif/video.
@madebr I've been playing with this got and https://github.com/prince-chrismc/conan-center-index-pending-review/pull/32/commits/1de7c841fe960c2789abd7a7af5f23195edb5d8f which is a loop of the last week of daily summaries
Thoughts?
kenneth-jia
commented
2 years ago Hi, @prince-chrismc
About the https://github.com/conan-io/conan-center-index/pull/11430 , previously it has been approved by @uilianries , while I pushed another small commit after while and re-required the review. Maybe that makes the "Approvers" column empty? Sorry, this is the first time I raised a PR to conan-conter-index and I'm not quite familiar with these procedures. Could you please help tell how long it might take to get the PR merged? 😅 Thanks!
prince-chrismc
commented
2 years ago this is the first time I raised a PR to
conan-conter-indexand I'm not quite familiar with these procedures.
First and foremost, welcome!
This bot is designed to "closely" follow the https://github.com/conan-io/conan-center-index/blob/master/docs/review_process.md along with the peculiarities of GitHub.
I pushed another small commit after while and re-required the review. Maybe that makes the "Approvers" column empty?
Yes, thats exactly how GitHub tracks approvals! When you push commit's you dismiss stale approvals.
C3i and this bot both count the approvals on the very last commit which is why they go away
Could you please help tell how long it might take to get the PR merged?
If only I could predict the future
Given the attention in the last 24hrs I'd say it should be the average 5 days, I noticed it's already been 20 days but it should not take much longer 🤞
Neustradamus
commented
2 years ago To follow this ticket :)
ericLemanissier
commented
2 years ago FYI, the same information is available as a full web page at https://prince-chrismc.github.io/conan-center-index-pending-review/
prince-chrismc
commented
1 year ago Just computing the table was 1250 requests https://github.com/prince-chrismc/conan-center-index-pending-review/actions/runs/4345764404/jobs/7590927150 😱 should be working again
:sparkles: Summary of Pull Requests Pending Review!
:ballot_box_with_check: Selection Criteria:
Legend
:nerd_face: Please Review!
There are 215 pull requests currently under way :detective:
:heavy_check_mark: Ready to Merge
Currently 2 pull requests are waiting to be merged :tada:
:clipboard: Statistics
Raw JSON data
:bar_chart: Open Versus Merged
Legend
Pull requests are depicted as:
For reference:
:hourglass: Time Spent in Review
If you are wondering how long it will take for you pull request to get merged; this graph should give you an idea.
Found this useful? Give it a :star: :pray: