-
Notifications
You must be signed in to change notification settings - Fork 10
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
update function names for run_fremake subcommand #246
Conversation
fre/make/gfdlfremake/checkout.py
Outdated
@@ -115,6 +115,7 @@ def run (self): | |||
Param: | |||
- self The checkout script object | |||
""" | |||
os.chmod(self.src+"/"+self.fname, 0o744) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this would be needed here. The checkout script being made executable is now handled in the createCheckout
tool:
fre-cli/fre/make/createCheckout.py
Line 78 in c826804
os.chmod(srcDir+"/checkout.sh", 0o744) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users want the checkout script to be made executable for the bare-metal build before actually using the execute
option (want the ability to run themselves). I made that change in the createCheckout.py
tool and forgot to change it for run_fremake
as well. I would put this chmod
line here:
fre-cli/fre/make/runFremake.py
Line 92 in 34eb37e
## TODO: Options for running on login cluster? |
Right after the checkout script gets created with the line freCheckout.finish(pc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fixed now with commit ee4b5b5. Will run-fremake not always execute everything? I think thats what it does currently.
''' | ||
Decorator for calling _fremake_run - allows the decorated version | ||
of the function to be separate from the undecorated version | ||
''' | ||
return _fremake_run(yamlfile,platform,target,parallel,jobs,no_parallel_checkout,verbose) | ||
return fremake_run(yamlfile,platform,target,parallel,jobs,no_parallel_checkout,verbose) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are supposed to be switched, great catch! Thank you
Describe your changes
Some changes needed to get the run-fremake subcommand working. Just switches around the names for
run_fremake
and adds a chmod.Checklist before requesting a review