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

opencascade: new variants #35345

Merged
merged 6 commits into from
Jun 12, 2023
Merged

opencascade: new variants #35345

merged 6 commits into from
Jun 12, 2023

Conversation

wdconinc
Copy link
Contributor

@wdconinc wdconinc commented Feb 5, 2023

OpenCascade has several major modules and not every application needs all of them. This adds variants for the various modules.

It also updates the 3rdparty dependency treatment.

@wdconinc
Copy link
Contributor Author

wdconinc commented Feb 5, 2023

@spackbot fix style

@spackbot-app
Copy link

spackbot-app bot commented Feb 5, 2023

Let me see if I can fix that for you!

@spackbot-app
Copy link

spackbot-app bot commented Feb 5, 2023

I was able to run spack style --fix for you!

spack style --fix
==> Running style checks on spack
  selected: isort, black, flake8, mypy
==> Modified files
  var/spack/repos/builtin/packages/opencascade/package.py
==> Running isort checks
  isort checks were clean
==> Running black checks
reformatted var/spack/repos/builtin/packages/opencascade/package.py
All done! ✨ 🍰 ✨
1 file reformatted.
  black checks were clean
==> Running flake8 checks
  flake8 checks were clean
==> Running mypy checks
Success: no issues found in 576 source files
  mypy checks were clean
==> spack style checks were clean
Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with style fixes.

wdconinc and others added 3 commits April 21, 2023 08:02
OpenCascade has several major modules and not every
application needs all of them. This adds variants for
the various modules.

It also uodatws the 3rdparty dependency treatment.
@wdconinc wdconinc force-pushed the opencascade-modules branch from 752c131 to abfff01 Compare April 21, 2023 13:02
@wdconinc wdconinc marked this pull request as ready for review April 21, 2023 13:02
@wdconinc wdconinc requested a review from alecbcs April 22, 2023 14:01
@wdconinc wdconinc requested a review from alecbcs April 26, 2023 19:19
@wdconinc
Copy link
Contributor Author

@spackbot fix style

@spackbot-app
Copy link

spackbot-app bot commented Apr 26, 2023

Let me see if I can fix that for you!

@spackbot-app
Copy link

spackbot-app bot commented Apr 26, 2023

I was able to run spack style --fix for you!

spack style --fix
==> Running style checks on spack
  selected: isort, black, flake8, mypy
==> Modified files
  var/spack/repos/builtin/packages/opencascade/package.py
==> Running isort checks
  isort checks were clean
==> Running black checks
reformatted var/spack/repos/builtin/packages/opencascade/package.py
All done! ✨ 🍰 ✨
1 file reformatted.
  black checks were clean
==> Running flake8 checks
  flake8 checks were clean
==> Running mypy checks
Success: no issues found in 578 source files
  mypy checks were clean
==> spack style checks were clean
Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with style fixes.

@wdconinc
Copy link
Contributor Author

FYI, the splitting into different modules is a first step towards making some of the dependencies dependent on the enabled modules. E.g. +vtk doesn't make sense unless +visualization, but +tbb does make sense for the foundation classes. I can either do all that work in this PR or in a different one. LMK.

alecbcs
alecbcs previously approved these changes Jun 11, 2023
Copy link
Member

@alecbcs alecbcs left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you for the contribution @wdconinc! Would you like to continue work on this PR to segment out the dependencies or add it in a follow up?

@wdconinc
Copy link
Contributor Author

I just compared my notes and there was one issue in this PR that I just fixed, but the rest is for a future PR (i.e. adding conditionals to the e.g. gl variants and dependencies). This is ready for merging.

Copy link
Member

@alecbcs alecbcs left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@alecbcs alecbcs enabled auto-merge (squash) June 12, 2023 16:16
@alecbcs alecbcs merged commit 316bfd8 into spack:develop Jun 12, 2023
@wdconinc wdconinc deleted the opencascade-modules branch July 9, 2023 21:15
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.

2 participants