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

ENG-15677-LMEval_QE_Updates #622

Merged
merged 3 commits into from
Jan 31, 2025
Merged

ENG-15677-LMEval_QE_Updates #622

merged 3 commits into from
Jan 31, 2025

Conversation

BSynRedhat
Copy link
Contributor

QE updates and tweaks for LM Eval content.

aduquett
aduquett reviewed Jan 30, 2025
View reviewed changes
Copy link
Contributor

@aduquett aduquett left a comment

Choose a reason for hiding this comment

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

Added a few suggestions. Should these all be procedure modules with numbered steps instead of reference modules?

modules/lmeval-evaluation-job.adoc Show resolved Hide resolved
modules/lmeval-evaluation-job.adoc Show resolved Hide resolved
modules/lmeval-evaluation-job.adoc Outdated Show resolved Hide resolved
modules/lmeval-evaluation-job.adoc Outdated Show resolved Hide resolved
modules/lmeval-evaluation-job.adoc Show resolved Hide resolved
modules/setting-up-lmeval.adoc Outdated Show resolved Hide resolved
modules/setting-up-lmeval.adoc Show resolved Hide resolved
modules/setting-up-lmeval.adoc Outdated Show resolved Hide resolved
modules/setting-up-lmeval.adoc Outdated Show resolved Hide resolved
modules/lmeval-scenarios.adoc
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the bullets under the Procedure sections be numbers instead?

Copy link
Contributor Author

@BSynRedhat BSynRedhat Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I wasn't sure about using numbers, as some sections don't really lend themselves to it (eg. they have one step which just outlines the code snippet they need to use (which follows the step) and then no second step, or one other step that is an optional step). But I have put in numbers. Thanks for the suggestion :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I steered you wrong here - single-step procedures should still use a bullet according to the IBM style guide. But multi-step should be numbered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No you were right, I should have considered this! I have changed the ones with one step back to bullets, and left the others with the updated numbered lists.

@BSynRedhat BSynRedhat force-pushed the 15677_QE_Updates_LMEval branch from 72a02c0 to 068756a Compare January 31, 2025 12:00
aduquett
aduquett approved these changes Jan 31, 2025
View reviewed changes
Copy link
Contributor

@aduquett aduquett left a comment

Choose a reason for hiding this comment

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

LGTM

@BSynRedhat BSynRedhat merged commit 20b9dca into opendatahub-io:main Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduquett aduquett aduquett approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@BSynRedhat @aduquett