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

move the getKey to getName #1856

Merged
merged 10 commits into from
Apr 29, 2021
Merged

move the getKey to getName #1856

merged 10 commits into from
Apr 29, 2021

Conversation

phlptp
Copy link
Member

@phlptp phlptp commented Apr 21, 2021

to make things a little clear as Key was causing some confusion

Summary

If merged this pull request will change ..getKey to getName and remove references to key in the api

Proposed changes

@phlptp phlptp added the v3 breaking change HELICS 3 API changes label Apr 21, 2021
@phlptp phlptp requested review from kdheepak and trevorhardy April 21, 2021 23:20
github-actions bot and others added 3 commits April 21, 2021 16:26
@github-actions @phlptp
Automated formatting of repo files (#1857)
4665571 
Co-authored-by: Philip Top <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@phlptp
fix cpplint issues and backup files
68deb2b
@github-actions @phlptp
Automated formatting of repo files (#1858)
93b9617 
Co-authored-by: Philip Top <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
nightlark
nightlark reviewed Apr 22, 2021
View reviewed changes
Copy link
Member

@nightlark nightlark left a comment

Choose a reason for hiding this comment

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

Just some docstrings using the word key

src/helics/shared_api_library/ValueFederate.h Outdated Show resolved Hide resolved
src/helics/shared_api_library/ValueFederate.h Outdated Show resolved Hide resolved
src/helics/shared_api_library/ValueFederate.h Outdated Show resolved Hide resolved
src/helics/shared_api_library/ValueFederate.h Outdated Show resolved Hide resolved
@kdheepak
Copy link
Contributor

This file has some ...GetKey calls that need to be changed to ...GetName:

HELICS/tests/helics/shared_library/ValueFederateTests1.cpp

Line 63 in 5f0ab12

CE(helicsPublicationGetKey(pubid, sv, HELICS_SIZE_MAX));

phlptp and others added 4 commits April 23, 2021 13:51
@phlptp @nightlark
Apply suggestions from code review
0df6f71 
Co-authored-by: Ryan Mast <[email protected]>
@phlptp
remove old unused test files
3ca751b
@phlptp
Merge branch 'getkey_to_getName' of https://github.com/GMLC-TDC/HELICS …
f42125f 
…into getkey_to_getName
@github-actions @nightlark @phlptp
Automated formatting of repo files [skip ci] (#1863)
595f232 
* Fix compress images action workflow syntax

* Exclude pushes to main

* Automated formatting of repo files

Co-authored-by: Ryan Mast <[email protected]>
Co-authored-by: Philip Top <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@kdheepak
Copy link
Contributor

LGTM!

@phlptp phlptp merged commit efa26c0 into helics3 Apr 29, 2021
@phlptp phlptp deleted the getkey_to_getName branch April 29, 2021 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nightlark nightlark nightlark left review comments

@kdheepak kdheepak kdheepak approved these changes

@trevorhardy trevorhardy trevorhardy approved these changes

Assignees
No one assigned
Labels
v3 breaking change HELICS 3 API changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@phlptp @kdheepak @nightlark @trevorhardy