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

TribitsAdjustPackageEnables.cmake: Use clean code ordering and more cleanup (#63) #547

Merged
merged 3 commits into from
Dec 12, 2022

Conversation

bartlettroscoe
Copy link
Member

Description

Now this module TribitsAdjustPackageEnables.cmake should be ready for the next set of refactorings and extensions to complete #63.

Instructions for reviewers

Best to review this PR commit-by-commit:

@bartlettroscoe bartlettroscoe changed the title ribitsAdjustPackageEnables.cmake: Use clean code ordering and more cleanup (#63) TribitsAdjustPackageEnables.cmake: Use clean code ordering and more cleanup (#63) Dec 10, 2022
Base automatically changed from 63-combined-package-data-structures-4 to master December 12, 2022 18:53
rabartlett1972
rabartlett1972 previously approved these changes Dec 12, 2022
Copy link
Collaborator

@rabartlett1972 rabartlett1972 left a comment

Choose a reason for hiding this comment

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

Since Kyle is out until mid Jan, I will allow this to merge and set up for a post-merge review.

Just saw this while working on #63 refactorings.
This ordering makes more sense compared to the reverse clean code ordering
that I usesd to use.  In CMake, you can call macros/functions defined after
the call in the source file (and this is true of most languages, even static
langauges like C++ due to forward declarations).
* Rename local vars to camelCase and pick better names for some vars

* Remove unused vars
@bartlettroscoe bartlettroscoe force-pushed the 63-combined-package-data-structures-5 branch from 094956d to b616ad3 Compare December 12, 2022 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants