-
-
Notifications
You must be signed in to change notification settings - Fork 280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Status Dashboard #2090
Status Dashboard #2090
Conversation
✅ Deploy Preview for conda-forge-previews ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
b7118fb
to
2ded2f6
Compare
2ded2f6
to
b716c86
Compare
The CI failure seems to come from trying to access a dynamic URL, but I am not certain. You can see in the deployment that the actual site logic is functional, but there might be something I am missing. |
Thanks a lot! As a "normal" contributor, I was wondering if in this new version there is a way to see the error messages for "non-solvable" packages in migration, see for example https://conda-forge.org/status/#libabseil20240116_libgrpc161_libprotobuf4252 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments with, mostly, tiny things. The biggest item is that there's no documentation in the code at all. While it's well written and it can be easily navigated in the IDE, I think it would be beneficial to have a big picture description on how the dashboard is built and under what principles. If possible, each module should have a top-level docstring (or whatever the equivalent is in JS) with the purpose and scope of the module.
package.json
Outdated
"@mdx-js/react": "^3.0.0", | ||
"chart.js": "^4.4.0", | ||
"chartjs-adapter-moment": "^1.0.1", | ||
"clsx": "^1.2.1", | ||
"install": "^0.13.0", | ||
"moment": "^2.29.4", | ||
"octokit": "^3.1.2", | ||
"prism-react-renderer": "^2.3.1", | ||
"react": "^18.2.0", | ||
"react-dom": "^18.2.0" | ||
"react-chartjs-2": "^5.2.0", | ||
"react-dom": "^18.2.0", | ||
"react-markdown": "^9.0.1", | ||
"remark-gfm": "^4.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it would be useful to document why these dependencies have been added. For example, is remark-gfm
directly used now? Does that mean we can use Github flavoured markdown in the documentation now? Or is it just because it made it easier to write certain parts of the dashboard?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coming back to this, it looks like what folks do is something along the lines of:
"packages": {
...
},
"// packages": {
"//": "There's nothing special about this dict; it's just a collection of comments",
"install": "This package is used for ...",
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how common this is in the JS world, but I'm missing the equivalent of module docstrings in almost all files. Having a little bit of technical documentation to see how to navigate the dashboard code would be super helpful.
Adding screenshot for clarity: |
I can't find that functionality either. The URL for that migration would be https://deploy-preview-2090--conda-forge-previews.netlify.app/status/migration/libabseil20240116_libgrpc161_libprotobuf4252. If I filter for unsolvable only, I just get: Weirdly, the package names in the first column are not clickable (they take you nowhere), despite having a hover state. Don't know if bug or intended, but the hover is confusing. |
You're right, this is a piece of functionality that is missing in the new version. I'll give it some thought and try to incorporate it. (cc: @GabrielaVives) |
I see what's happened. Almost all the left-column items have a PR link. Some do not, but they're being automatically linked to empty string. I will put in a check to only link the ones that actually have a URL for the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot find the solver errors we post to the status page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need to integrate a large banner at the top with the status repo issues tracker.
Really neat! I have some feedback on the migrations views:
|
Co-authored-by: jaimergp <[email protected]>
Wow I did not know about this feature! |
The error is:
Which is expected because this PR hasn't been merged yet so it's actually checking the old single page
|
.status_dashboard_toc { | ||
border: var(--ifm-table-border-width) solid #ccc; | ||
border-radius: 5px; | ||
font-size: 16px; | ||
position: sticky; | ||
top: 80px; | ||
max-width: 250px; | ||
} | ||
|
||
.status_dashboard_toc a { | ||
display: inline-block; | ||
min-width: 100%; | ||
text-decoration: none; | ||
} | ||
|
||
.status_dashboard_toc ul { | ||
list-style-type: none; | ||
margin: 0; | ||
padding: 0; | ||
} | ||
|
||
.status_dashboard_toc li { | ||
background-color: var(--ifm-table-stripe-background); | ||
border-radius: 5px; | ||
padding: 1px 5px; | ||
user-select: none; | ||
} | ||
|
||
.status_dashboard_toc li:hover { | ||
background-color: transparent; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rework this styling so it matches the sidebar in /docs
? It's almost there but there are subtle differences. Otherwise, we could also try the simple link list of /blog
.
I've also noticed that the migration tables can only be sorted together; e.g. if I sort "long-running migrations" by name, "Regular migrations" and "Closed migrations" also get sorted in the same way: Screen.Recording.2024-02-27.at.10.20.34.mov |
These are not separate tables. They are one table with different
subheadings. This sorting behavior is by design. Does it harm usability to
sort the entire table by the column you’re interested in?
…On Tue, Feb 27, 2024 at 09:23 jaimergp ***@***.***> wrote:
I've also noticed that the migration tables can only be sorted together;
e.g. if I sort "long-running migrations" by name, "Regular migrations" and
"Closed migrations" also get sorted in the same way:
https://github.com/conda-forge/conda-forge.github.io/assets/2559438/511d983b-e051-4c58-a095-fd47440ebeea
—
Reply to this email directly, view it on GitHub
<#2090 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABG6KLHWVW2AUUDVBBHFZTYVWQZFAVCNFSM6AAAAABD2MSLPOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRWGEYTQOJQGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Not necessarily. I was expecting individual behavior given the sort buttons in each subtable, but if it makes everything more complicated, for me it's not personally worth "fixing" it. |
I think that what is counter-intuitive here, is the repetitive column headers that give the impression that there are 3 independent tables. |
Maybe you can use https://docusaurus.io/docs/api/themes/configuration#use-color-mode? |
More dark mode visibility issues, but I think these can be mostly addressed with Gabriela's suggestion at #2090 (comment) |
I think I can use this, yes. I'll give a try! Thanks @jaimergp! And yes please, if merge conflicts arise that you know the right version of, please help! I'll make a comment in this thread when I have resolved the theme switching issue. |
3eeecfa
to
92e2081
Compare
The theme switching now uses the color mode helper. It still needs to – unfortunately – compute the style value of the CSS color variables once at runtime (which is also why there are two new CSS variables that are invariant and define light and dark primary colors and are used to populate the respective dark/light :root {
/*
These two variables are defined in root and used at runtime for calculating
the light mode and dark mode values of the primary color variable in order
to populate `backgroundColor` prop `<UsageChart>`.
*/
--ifm-color-primary-light-mode: #008478;
--ifm-color-primary-dark-mode: #4db6ac;
/* ... other stuff ... */
--ifm-color-primary: var(--ifm-color-primary-light-mode);
/* ... other stuff ... */
}
[data-theme="dark"] {
--ifm-color-primary: var(--ifm-color-primary-dark-mode);
/* ... other stuff ... */
} Updating the colors of the details page, I would propose doing this as a follow-on. |
Thanks a bunch! Will merge by EOD. |
It's live! https://conda-forge.org/status |
Looks like we lost the migration table's "Done" column (please see screenshot below). It is already in the details page Adding it back in PR: #2114 Please let us know if anything else is needed |
How long does it take for new migrations to get added to the status page, resp. must happen for it to be picked up? I cannot see a migration for libsentencepiece, 24h after merging conda-forge/conda-forge-pinning-feedstock#5644... |
More likely this is a bot bug than a webpage bug The webpage is just reading the bot's data The migration referenced doesn't show up in the bot's data Would suggest raising a new bot issue |
Can someone who was involved in this effort take a look at #2137 please? Looking at a big migration like python312 with >500 open PRs, it's impossible to tell where the biggest bottlenecks are currently. We need to know the number of total children, not just the immediate ones, otherwise shepherding such big migrations becomes basically impossible. |
This is an updated status/dashboard page for the conda-forge website.
This PR covers the functionality every core component of the current conda-forge status site.
Please TRY IT OUT, feedback is welcome 🙏
cf. #1971