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.

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

Add missing header to mechanical_forces_op_test.h #395

Closed
wants to merge 1 commit into from

Conversation

imorlxs
Copy link
Member

@imorlxs imorlxs commented Sep 7, 2024

We use NewOperation() in this file, so we should add the header where the function is definded.

We use NewOperation() in this file, so we should add the header where the function is definded.
Copy link

sonarqubecloud bot commented Sep 7, 2024

@TobiasDuswald
Copy link
Contributor

Do you think you can bundle all of these PRs (#390 - #395) that add one line of code in one PR? @imorlxs

Maybe even consider just adding these changes to #385 as I understood this is the driving reason for doing so ?

@imorlxs
Copy link
Member Author

imorlxs commented Sep 9, 2024

Do you think you can bundle all of these PRs (#390 - #395) that add one line of code in one PR? @imorlxs

Maybe even consider just adding these changes to #385 as I understood this is the driving reason for doing so ?

I have bundled all these changes in PR #388. I split them into separate PRs in case the team wanted to know the reasons for adding all these lines. I can close these PRs and reopen #388.

The changes have been added to PR #385, but I use that PR as draft only to use GitHub Actions. Later, I plan to split all the changes into smaller PRs to make the review process easier.

@TobiasDuswald
Copy link
Contributor

Appreciate the thought of splitting the different PRs for easier review but I just took a look at #385 and there are not that many changes so don't worry about that. I'd suggest you just keep everything in that branch / PR such that all code related to this change of the build system can be found in one PR. Once the PR is ready, we can start the review process. If we had to come back to that development effort in a year from now, we have everything together in one place, i.e.., single commit / PR.

@imorlxs
Copy link
Member Author

imorlxs commented Sep 10, 2024

My plan was to create an issue to monitor the progress of the C++ Modules' implementation and to submit incremental pull requests containing self-contained changes. This approach would allow us to revisit the development effort and keep a clear record of all modifications. However, if you prefer the approach of a single pull request, I am okay with doing it that way.

@TobiasDuswald
Copy link
Contributor

Great, let's stick to the single PR then! Thanks a lot @imorlxs , we appreciate your help on this!

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.

2 participants