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

Update SET, schedule and tag APIs in the C target #1097

Merged
merged 43 commits into from
Apr 28, 2022
Merged

Update SET, schedule and tag APIs in the C target #1097

merged 43 commits into from
Apr 28, 2022

Conversation

housengw
Copy link
Contributor

@housengw housengw commented Apr 12, 2022 •
edited
Loading

Please refer to issue #1108 for the names of the APIs before and after the change.

@housengw housengw mentioned this pull request Apr 12, 2022
Merged
@housengw housengw added c Related to C target testing labels Apr 12, 2022
@housengw housengw self-assigned this Apr 12, 2022
@housengw housengw changed the title Add SET_DYNAMIC API and tests Add SET_DYNAMIC API to the C target and tests Apr 12, 2022
edwardalee
edwardalee requested changes Apr 13, 2022
View reviewed changes
Copy link
Collaborator

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

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

Small error detected. Approve modulo changing function names to lower case.

test/C/src/SetWithCustomDestructor.lf Outdated Show resolved Hide resolved
test/C/src/SetWithCustomDestructor.lf Outdated Show resolved Hide resolved
housengw and others added 2 commits April 12, 2022 18:50
@housengw @edwardalee
Apply suggestions from code review
da02ec9 
Co-authored-by: Edward A. Lee <eal@eecs.berkeley.edu>
@housengw
update submodule pointer
f525540
@housengw
Copy link
Contributor Author

Seems like a Windows specific problem is keeping SetWithCustomConstructor from passing on Windows

@housengw housengw force-pushed the set-destructor-variant branch from 323b8d7 to 012a669 Compare April 18, 2022 20:29
@housengw housengw changed the title Add SET_DYNAMIC API to the C target and tests Make SET in C target work with both token and non-token types Apr 18, 2022
@housengw housengw requested a review from edwardalee April 18, 2022 23:53
@housengw housengw marked this pull request as draft April 19, 2022 00:14
@housengw housengw changed the title Make SET in C target work with both token and non-token types Update SET, schedule and tag APIs in the C target Apr 25, 2022
@housengw housengw marked this pull request as ready for review April 25, 2022 17:25
Soroosh129
Soroosh129 approved these changes Apr 26, 2022
View reviewed changes
Copy link
Contributor

@Soroosh129 Soroosh129 left a comment

Choose a reason for hiding this comment

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

LGTM!

Before merging this, let's make sure everyone is aware of this change (maybe by announcing it on lf-interest) and that the website and/or the wiki are up-to-date.

@housengw
Copy link
Contributor Author

I can make an announcement. With regards to the website, I think we should merge the PR first. Otherwise we would have documentation that refer to code not in master

@housengw housengw linked an issue Apr 26, 2022 that may be closed by this pull request
Tracking the renaming of APIs in C and Python target #1108
Closed
edwardalee
edwardalee approved these changes Apr 26, 2022
View reviewed changes
Copy link
Collaborator

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

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

LGTM! We can immediately update the website repo because that website is not live. But anyway, I think we should forge ahead with this...

test/C/src/SetCopyConstructor.lf Outdated Show resolved Hide resolved
test/C/src/SetCopyConstructor.lf Outdated Show resolved Hide resolved
test/C/src/SetDestructor.lf Outdated Show resolved Hide resolved
test/C/src/include/array.h Show resolved Hide resolved
test/C/src/include/hello.h Show resolved Hide resolved
test/C/src/SetArray.lf Outdated Show resolved Hide resolved
@housengw housengw merged commit 7decdbd into master Apr 28, 2022
This was referenced Apr 28, 2022
Merged
Merged
Merged
@housengw housengw deleted the set-destructor-variant branch May 2, 2022 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Soroosh129 Soroosh129 Soroosh129 approved these changes

@edwardalee edwardalee edwardalee approved these changes

Assignees

@housengw housengw

Labels
c Related to C target testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tracking the renaming of APIs in C and Python target
3 participants
@housengw @Soroosh129 @edwardalee