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.

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

cleanup: iterate on pr/47 onload-dev clarification #48

Merged
merged 2 commits into from
Oct 3, 2024
Merged

Conversation

abower-amd
Copy link
Collaborator

Apologies for the second PR. I've taken @dchadwic-xilinx's suggestion, reworded commit log (no point in issue number that doesn't exist) then reordered the release notes and woven the onload-devel theme through.

@abower-amd abower-amd requested a review from a team October 3, 2024 15:49
@abower-amd abower-amd requested a review from a team as a code owner October 3, 2024 15:49
Comment on lines +26 to +42
New onload development package
------------------------------

Onload now includes a package containing headers required by ef_vi
applications like TCPDirect. Installing this package is required to
build TCPDirect and TCPDirect applications.

Please ensure the openonload-devel RPM or onload-dev DEB is installed
after building Onload before attempting to build TCPDirect.


Public Onload/ef_vi control plane API
-------------------------------------

The Onload control plane in this Onload-9.0.0 is presented via a new
public API that can be used by ef_vi applications. As an ef_vi application,
TCPDirect now uses this API to query the control plane server.
Copy link
Contributor

@dchadwic-xilinx dchadwic-xilinx Oct 3, 2024

Choose a reason for hiding this comment

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

Can we swap the order of these two sections? If a customer gets stuck on the missing onload-dev message when building, their first thought may be "Did I miss a step?" so would be best if its immediately before the installation section imo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I already switched it to this way round to have the new package section flow naturally from

The packaging for Onload and TCPDirect has been refreshed to allow suitable new build and installation workflows for the TCPDirect source package."

Do you really think the idea hasn't been called out enough by the time they reach the installation instructions and the very verbose point 0?

Copy link
Contributor

Choose a reason for hiding this comment

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

Its more because it answers the question of where a customer gets onload-dev from:

Please ensure the openonload-devel RPM or onload-dev DEB is installed
after building Onload before attempting to build TCPDirect.

This sentence says you get it from building Onload (rather than some other source).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we are in the margins of extra utility here. The main obstacle to your initial experience was that we had failed to generate the release notes adjacent to the package you trialed!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thats likely true. If you are happy with the current order, then that is fine I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks Daniel!

@abower-amd abower-amd merged commit 03cff7f into v9_0 Oct 3, 2024
2 checks passed
@abower-amd abower-amd deleted the pr/47 branch October 3, 2024 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants