Skip to content

Remove CommParams and make the fields there data attributes.#2256

Merged
wujingyue merged 4 commits intomainfrom
wjy/attribute
Jun 3, 2024
Merged

Remove CommParams and make the fields there data attributes.#2256
wujingyue merged 4 commits intomainfrom
wjy/attribute

Conversation

@wujingyue
Copy link
Collaborator

@wujingyue wujingyue commented May 15, 2024

I initially intended to just make CommParams a data attribute. Then I realized the extra indirection of CommParams doesn't seem to give much practical value after various cleanups we've done, so I flattened CommParams as well and that saved a bit of typing.

Team team_;
};

// The method "post" triggers the execution of the communication. This call is
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: I moved this comment to postSingleCommunication, because Communication::post is no longer a thing.

@wujingyue
Copy link
Collaborator Author

!build --dist

@wujingyue wujingyue requested review from cowanmeg and samnordmann May 15, 2024 23:39
@xwang233
Copy link
Collaborator

!build --dist

Copy link
Collaborator

@samnordmann samnordmann left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks, this makes Communication IRs cleaner

@wujingyue wujingyue force-pushed the wjy/underscore branch 4 times, most recently from 48680ed to 53cc014 Compare June 3, 2024 18:38
Base automatically changed from wjy/underscore to main June 3, 2024 19:45
@wujingyue
Copy link
Collaborator Author

!build

@wujingyue wujingyue merged commit d71cd34 into main Jun 3, 2024
@wujingyue wujingyue deleted the wjy/attribute branch June 3, 2024 23:16
zasdfgbnm pushed a commit that referenced this pull request Jun 5, 2024
I initially intended to just make CommParams a data attribute. Then I
realized the extra indirection of CommParams doesn't seem to give much
practical value after various cleanups we've done, so I flattened
CommParams as well and that saved a bit of typing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants