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

[cling-cpt] Made a new build option for last-stable and current-dev [skip-ci] #11019

Merged
merged 4 commits into from
Aug 13, 2022

Conversation

saisoma123
Copy link
Contributor

This Pull request: Allows for users to have the option of just building the CPT not packaging it.

Changes or fixes: Added a current-dev-build and last-stable-build option in the argument parser, and added additional if statements if it is the normal current-dev or last-stable option, so that there is also packaging like normal.

Checklist:

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

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

@phsft-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@vgvassilev
Copy link
Member

Can you provide a rationale of the new options and in which situations they are useful?

@saisoma123
Copy link
Contributor Author

@vgvassilev just added a rationale to the first commit message

@saisoma123 saisoma123 force-pushed the buildOpt branch 2 times, most recently from ad79f3b to da27411 Compare July 30, 2022 22:39
LLVM_GIT_URL = args['with_llvm_url']
else:
LLVM_GIT_URL = "http://root.cern.ch/git/llvm.git"

if args['with_binary_llvm'] and args['with_llvm_tar']:

Copy link
Member

Choose a reason for hiding this comment

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

Seems a stray space.

travis_fold_start("git-clone")
current_cond = args['current_dev'] if args['current_dev'] else args['current_dev_build']
Copy link
Member

Choose a reason for hiding this comment

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

Can we use a more meaningful name here? Is this meant to say current_branch?

cleanup()

elif args['current_dev'] == 'deb' or (args['current_dev'] == 'pkg' and DIST == 'Ubuntu'):
elif current_cond == 'deb' or (current_cond == 'pkg' and DIST == 'Ubuntu'):
Copy link
Member

Choose a reason for hiding this comment

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

Likewise. Is this meant to say current_packaging_mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vgvassilev Yeah just fixed everything thanks for letting me know

I added this option because it was listed as an
improvement that could be made to the CPT
on the meta issue list. These options are nice to
have because sometimes the user might just want
to build Cling and not package it, for fast
testing and usage.
@vgvassilev vgvassilev merged commit 6018b5f into root-project:master Aug 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants