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

Added new starting point for cpt.py- Cling Packaging Tool [skip-ci] #10728

Merged
merged 1 commit into from
Jun 28, 2022

Conversation

saisoma123
Copy link
Contributor

This Pull request:

Adds a new starting point for cpt.py

Changes or fixes:

Added if name == "main" block to cpt.py

Checklist:

  • tested changes locally
  • [NA] updated the docs (if necessary)

This PR fixes 406 (root-project/cling#406)

@phsft-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@Axel-Naumann Axel-Naumann requested review from vgvassilev and removed request for Axel-Naumann June 10, 2022 07:25
@Axel-Naumann Axel-Naumann changed the title Added new starting point for cpt.py- Cling Packaging Tool Added new starting point for cpt.py- Cling Packaging Tool [skip-ci] Jun 10, 2022
@@ -1834,7 +1834,8 @@ def custom_input(prompt, always_yes=False):
# Global variables #
###############################################################################

workdir = os.path.abspath(os.path.expanduser(args['with_workdir']))
if __name__ == "__main__":
Copy link
Member

Choose a reason for hiding this comment

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

Can you clarify (in the commit message) what this change does and why it is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @vgvassilev, I just made changes to the original commit message

Copy link
Member

Choose a reason for hiding this comment

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

Can you format your commit message according to https://cbea.ms/git-commit/

Copy link
Contributor Author

@saisoma123 saisoma123 Jun 19, 2022

Choose a reason for hiding this comment

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

Yep just reformatted according to the link @vgvassilev, sorry for the wait

Copy link
Member

Choose a reason for hiding this comment

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

I still see the old commit message style.

Copy link
Member

Choose a reason for hiding this comment

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

ping.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry not getting the notifications for this PR for some reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @vgvassilev, commit style fixed sorry for the wait

Added new main block because this was mentioned in the meta issue
list (root-project#406 in cling) as one of the rewrite steps. This also allows for
the main code to be run as opposed to running the main code plus the
function defintions.
Copy link
Member

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

LGTM!

@vgvassilev vgvassilev merged commit 3e5a873 into root-project:master Jun 28, 2022
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.

3 participants