Skip to content

Conversation

awlauria
Copy link
Contributor

@awlauria awlauria commented Sep 1, 2022

No description provided.

Move the parsing to autogen time, so configure can
grab it and export it.

Signed-off-by: Austen Lauria <awlauria@us.ibm.com>
@awlauria awlauria force-pushed the minor_version_cleanup branch from d64407c to 3b8150e Compare September 1, 2022 19:55
@awlauria awlauria marked this pull request as draft September 1, 2022 20:02
Copy link
Member

@jsquyres jsquyres left a comment

Choose a reason for hiding this comment

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

I think you don't want to change most of the "MPI-3.1" references in the man pages, because they refer to specific sections in the MPI-3.1 document, which may (did) change in other versions of the MPI document.

@awlauria
Copy link
Contributor Author

awlauria commented Sep 1, 2022

@jsquyres The rationale for a lot of these is once we claim v4.0 support, we would update these references to the new v4.0 sections if they change. Right now we claim v3.1 support, so it makes sense that we reference the v3.1 sections, but I can change any that don't make sense to change.

@jsquyres
Copy link
Member

jsquyres commented Sep 1, 2022

@jsquyres The rationale for a lot of these is once we claim v4.0 support, we would update these references to the new v4.0 sections if they change. Right now we claim v3.1 support, so it makes sense that we reference the v3.1 sections, but I can change any that don't make sense to change.

I hear what you're saying, but you'll somehow also need to update the section numbers for the MPI-4.0 doc, too. Otherwise, the content becomes factually incorrect.

That being said, even though it's "odd", it's technically correct to still cite the MPI-3.1 doc and a specific section in it: that's a fixed reference, and is still correct, even if Open MPI supports MPI-4.0.

@awlauria
Copy link
Contributor Author

awlauria commented Sep 1, 2022

@jsquyres The rationale for a lot of these is once we claim v4.0 support, we would update these references to the new v4.0 sections if they change. Right now we claim v3.1 support, so it makes sense that we reference the v3.1 sections, but I can change any that don't make sense to change.

I hear what you're saying, but you'll somehow also need to update the section numbers for the MPI-4.0 doc, too. Otherwise, the content becomes factually incorrect.

That being said, even though it's "odd", it's technically correct to still cite the MPI-3.1 doc and a specific section in it: that's a fixed reference, and is still correct, even if Open MPI supports MPI-4.0.

Correct - before upping the version to v4.0 (this PR does not change the version - just makes sure it's propagated) - these references will need to be updated to the correct section. I figured that is just something we should do anyway, once we claim v4.0 support and are ready to bump the actual version. 🤷

Anyway - I can revert this commit if is more trouble than it is worth.

@awlauria awlauria force-pushed the minor_version_cleanup branch from 3b8150e to 7e195fb Compare September 1, 2022 20:44
@awlauria awlauria marked this pull request as ready for review September 1, 2022 20:59
@jsquyres
Copy link
Member

jsquyres commented Sep 1, 2022

Anyway - I can revert this commit if is more trouble than it is worth.

Let's remove all the places where you changed "MPI-3.1 section xyz...", and leave that as a fixed, hard-coded reference. There were a few places where it was ok to update to MPI-|mpi_version|.|mpi_subversion|.

@awlauria awlauria force-pushed the minor_version_cleanup branch from 7e195fb to bf1e3ad Compare September 2, 2022 12:35
@awlauria
Copy link
Contributor Author

awlauria commented Sep 2, 2022

Anyway - I can revert this commit if is more trouble than it is worth.

Let's remove all the places where you changed "MPI-3.1 section xyz...", and leave that as a fixed, hard-coded reference. There were a few places where it was ok to update to MPI-|mpi_version|.|mpi_subversion|.

Done

@awlauria
Copy link
Contributor Author

awlauria commented Sep 9, 2022

@jsquyres is this good to go?

@awlauria awlauria requested a review from jsquyres September 9, 2022 15:09
Signed-off-by: Austen Lauria <awlauria@us.ibm.com>
@awlauria awlauria force-pushed the minor_version_cleanup branch from bf1e3ad to abe62d2 Compare September 9, 2022 17:17
@awlauria
Copy link
Contributor Author

awlauria commented Sep 9, 2022

@jsquyres I made that change, but you might want to refresh as it seems you might be looking at a stale version. The spawn_multiple file is not touched anymore.

@awlauria
Copy link
Contributor Author

awlauria commented Sep 9, 2022

bot:aws:retest

@awlauria awlauria requested a review from jsquyres September 12, 2022 15:22
@awlauria awlauria merged commit b2c26c5 into open-mpi:main Sep 12, 2022
@awlauria awlauria deleted the minor_version_cleanup branch September 12, 2022 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants