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

Removing njoy21 library creation. #112

Merged
merged 1 commit into from
Aug 12, 2020
Merged

Conversation

jlconlin
Copy link
Member

This is an attempt to remove the extra step of creating an NJOY21 library, which doesn't really make sense.

Copy link
Contributor

@nathangibson14 nathangibson14 left a comment

Choose a reason for hiding this comment

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

I'm sure the reason NJOY21 had a library and an executable previously was because of the tests. This looks like it should work to me, but let me make sure of that before I approve.

There are some tweaks/clean up that could be done to the CMakeLists.txt file, but there's no need to do that when I'll be re-updating them soon enough anyway.

@jlconlin
Copy link
Member Author

I'm sure the reason NJOY21 had a library and an executable previously was because of the tests. This looks like it should work to me, but let me make sure of that before I approve.

There are some tweaks/clean up that could be done to the CMakeLists.txt file, but there's no need to do that when I'll be re-updating them soon enough anyway.

This just reduces the cycle time for me so it was worth it to play around and see if I could speed things up.

@nathangibson14 nathangibson14 self-requested a review August 12, 2020 22:32
Copy link
Contributor

@nathangibson14 nathangibson14 left a comment

Choose a reason for hiding this comment

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

It works for me.

@jlconlin jlconlin merged commit 49dd6e2 into feature/RECONR Aug 12, 2020
@whaeck whaeck deleted the feature/no-library branch November 2, 2022 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants