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

[Core] Disable runkratos #9734

Closed
wants to merge 8 commits into from
Closed

[Core] Disable runkratos #9734

wants to merge 8 commits into from

Conversation

roigcarlo
Copy link
Member

📝 Description
Disabling and deprecating runkratos. See #9733

🆕 Changelog

  • Disabling runkratos

@roigcarlo roigcarlo requested a review from a team as a code owner March 2, 2022 10:02
@EduardGomezEscandell
Copy link
Contributor

You may also want to remove it from the README

philbucher
philbucher reviewed Mar 4, 2022
View reviewed changes
kratos/python_scripts/run_tests.py Outdated
@@ -177,7 +177,7 @@ def print_summary(exit_codes):

def main():
# Define the command
cmd = os.path.join(os.path.dirname(KtsUtls.GetKratosMultiphysicsPath()), 'runkratos')
cmd = sys.executable
Copy link
Member

Choose a reason for hiding this comment

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

Please use the fct from testing.utilities

Copy link
Member Author

Choose a reason for hiding this comment

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

mm I've been looking for it but There seems to be no function to do this, which one are you suggestion?

Copy link
Member

Choose a reason for hiding this comment

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

Kratos/kratos/python_scripts/testing/utilities.py

Lines 9 to 14 in 9ecfcec

def GetPython3Command():
"""Return the name of the python command, can be used with subprocess."""
sys_executable = sys.executable
if sys_executable: # was found
return sys_executable
raise Exception("The python command could not be determined!")

Copy link
Member Author

Choose a reason for hiding this comment

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

aaa thx!

roigcarlo
roigcarlo commented Mar 11, 2022
View reviewed changes
CMakeLists.txt
message("-- No INSTALL_RUNKRATOS is defined, setting to ON")
set(INSTALL_RUNKRATOS OFF)
endif(NOT DEFINED INSTALL_RUNKRATOS)
OPTION ( INSTALL_RUNKRATOS "Installing deprecated runkratos" OFF )
Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, didn't know about this.

We should change all the ifs we have with this !!

philbucher
philbucher previously approved these changes Mar 15, 2022
View reviewed changes
@roigcarlo roigcarlo mentioned this pull request Sep 1, 2022
Open
19 tasks
@loumalouomega
Copy link
Member

Okay, I already did a branch with this..., I will remove the branch

@loumalouomega
Copy link
Member

Okay, I already did a branch with this..., I will remove the branch

It was this one... https://github.com/KratosMultiphysics/Kratos/tree/core/remove-runkratos

@loumalouomega loumalouomega changed the title Core/disable runkratos [Core] Disable runkratos Sep 9, 2022
@roigcarlo
Copy link
Member Author

It does not remove runkratos, only hides it. Also seems that I deleted more than intended. I will fix asap

@roigcarlo
Copy link
Member Author

No longer relevant. See #11513

@roigcarlo roigcarlo closed this Aug 28, 2023
@roigcarlo roigcarlo deleted the core/disable-runkratos branch January 19, 2024 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philbucher philbucher philbucher left review comments

Assignees
No one assigned
Labels
API Breaker Behaviour Change changes behaviour but not the API Cleanup Kratos Core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@roigcarlo @EduardGomezEscandell @loumalouomega @philbucher