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

Compile definitions no longer hardcoded in generated CMakeLists.txt #1622

Merged
merged 3 commits into from
Mar 12, 2023
Merged

Compile definitions no longer hardcoded in generated CMakeLists.txt #1622

merged 3 commits into from
Mar 12, 2023

Conversation

petervdonovan
Copy link
Collaborator

This is redundant to passing them on the command line. This change is in line with my long-term goal of disentangling the code generator from the build system.

@petervdonovan
Do not hardcode compile definitions.
7b7ad06 
This is redundant to passing them on the command line.
lhstrh
lhstrh reviewed Mar 4, 2023
View reviewed changes
Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

Did you mean to push changes to reactor-rs?

@petervdonovan petervdonovan marked this pull request as draft March 4, 2023 06:24
@petervdonovan
Update submodules.
61f8369 
The previous commit accidentally changed the C and Rust submodule
versions.
@petervdonovan
Update submodule.
17dc4ab
@petervdonovan
Copy link
Collaborator Author

Is this OK to merge now? The rust test failure is just a network issue.

@lhstrh
Copy link
Member

lhstrh commented Mar 10, 2023

Is this OK to merge now? The rust test failure is just a network issue.

There is a little more to it, actually. Let me have a closer look.

@lhstrh
Copy link
Member

lhstrh commented Mar 10, 2023

There was a warning related to the Gradle configuration not being found in the root of the workspace, but that's not actually armful.

@lhstrh lhstrh marked this pull request as ready for review March 10, 2023 16:17
@lhstrh
Copy link
Member

lhstrh commented Mar 10, 2023

Where is the reactor-c PR that should be merged along with this?

@petervdonovan petervdonovan mentioned this pull request Mar 10, 2023
Merged
@lhstrh
Copy link
Member

lhstrh commented Mar 10, 2023

lf-lang/reactor-c#179 only touches 1 file, but the summary for the changes in reactor-c in this PR lists many more changes. Why?

@petervdonovan
Copy link
Collaborator Author

the summary for the changes in reactor-c in this PR lists many more changes. Why?

Probably because the submodule version that we point to in master is out of date. I just now double-checked the submodule hash in this branch, and it is indeed 158be58

@petervdonovan petervdonovan merged commit b0e3a70 into master Mar 12, 2023
@petervdonovan petervdonovan deleted the cmake-cleanup branch March 12, 2023 04:51
@petervdonovan petervdonovan added c Related to C target runtime Related to the runtime implementation build system cleanup labels Mar 12, 2023
@petervdonovan petervdonovan changed the title Do not hardcode compile definitions in generated CMakeLists.txt Compile definitions no longer hardcoded in generated CMakeLists.txt Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lhstrh lhstrh lhstrh approved these changes

Assignees
No one assigned
Labels
build system c Related to C target cleanup runtime Related to the runtime implementation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@petervdonovan @lhstrh