Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update TOC page #3580

Merged
merged 7 commits into from
Jan 15, 2024
Merged

Update TOC page #3580

merged 7 commits into from
Jan 15, 2024

Conversation

MVarshini
Copy link
Contributor

PBENCH-1216

Update TOC page

@MVarshini MVarshini added the Dashboard Of and relating to the Dashboard GUI label Dec 1, 2023
@MVarshini MVarshini added this to the v0.73 milestone Dec 1, 2023
@MVarshini MVarshini requested review from webbnh and dbutenhof December 1, 2023 10:11
@MVarshini MVarshini self-assigned this Dec 1, 2023
dbutenhof
dbutenhof previously approved these changes Dec 1, 2023
View reviewed changes
Copy link
Member

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be happy to get this in as-is, but I'm throwing out a few minor suggestions anyway.

dashboard/src/actions/tocActions.js Outdated Show resolved Hide resolved
dashboard/src/actions/tocActions.js Outdated Show resolved Hide resolved
webbnh

This comment was marked as resolved.

Sign in to view

webbnh
webbnh reviewed Jan 2, 2024
View reviewed changes
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvements.

dashboard/src/actions/tocActions.js Outdated Show resolved Hide resolved
dbutenhof
dbutenhof previously approved these changes Jan 9, 2024
View reviewed changes
webbnh

This comment was marked as resolved.

Sign in to view

@MVarshini MVarshini requested review from webbnh and dbutenhof January 10, 2024 15:47
dbutenhof
dbutenhof previously approved these changes Jan 10, 2024
View reviewed changes
webbnh

This comment was marked as resolved.

Sign in to view

dbutenhof
dbutenhof reviewed Jan 10, 2024
View reviewed changes
Copy link
Member

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I agree that the nested search loop for size is awkward and inefficient, I just approved to get this out of the way. But if Varshini wants to improve that, it'd be great. 😁

dashboard/src/actions/tocActions.js Outdated Show resolved Hide resolved
@MVarshini MVarshini dismissed stale reviews from webbnh and dbutenhof via 82b9814 January 11, 2024 14:04
webbnh

This comment was marked as resolved.

Sign in to view

@MVarshini
Copy link
Contributor Author

Although the default parameter values for fetchTOC() happen to work, I don't think we should be depending on that. (I'm not blocking the merge over this, but I'm not approving, either....)

Got it. Updated the parameter list of fetchTOC()

webbnh
webbnh approved these changes Jan 15, 2024
View reviewed changes
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No more concerns. 🙂

dashboard/src/modules/components/TableOfContent/index.jsx Show resolved Hide resolved
@dbutenhof dbutenhof merged commit aaa842a into distributed-system-analysis:main Jan 15, 2024
4 checks passed
webbnh pushed a commit that referenced this pull request Jan 19, 2024
@MVarshini @webbnh
Update TOC page (#3580)
9be7689 
* PBENCH-1216
TOC page update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbutenhof dbutenhof dbutenhof approved these changes

@webbnh webbnh webbnh approved these changes

Assignees

@MVarshini MVarshini

Labels
Dashboard Of and relating to the Dashboard GUI
Projects
v0.72
Status: Done
Milestone
v0.73
Development

Successfully merging this pull request may close these issues.
3 participants
@MVarshini @webbnh @dbutenhof