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.

Sign up for GitHub

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

Jump to bottom

#377: Fixing typos #388

Merged
merged 3 commits into from
Aug 12, 2021
Merged

#377: Fixing typos #388

merged 3 commits into from
Aug 12, 2021

Conversation

marcinwrobel1986
Copy link
Collaborator

@marcinwrobel1986 marcinwrobel1986 commented Jul 15, 2021 •
edited by bartlettroscoe
Loading

Addresses #377

This PR:

  • fixes text typos in:
    • tribits/core/package_arch/TribitsReadAllProjectDepsFilesCreateDepsGraph.cmake
    • tribits/core/package_arch/TribitsReadDepsFilesCreateDepsGraph.cmake
    • tribits/core/package_arch/TribitsSystemDataStructuresMacrosFunctions.rst
  • there are still typos in functions and variables names, but I would leave that to @bartlettroscoe

@marcinwrobel1986 marcinwrobel1986 self-assigned this Jul 15, 2021
@bartlettroscoe bartlettroscoe mentioned this pull request Jul 15, 2021
Closed
2 tasks
@bartlettroscoe
Copy link
Member

there are still typos in functions and variables names, but I would leave that to @bartlettroscoe

@marcinwrobel1986, please go ahead and make any name changes you want in this PR. I will make sure nothing breaks before the merge.

@marcinwrobel1986 marcinwrobel1986 changed the title #377: - fixed text typos #377: Fixing typos Jul 15, 2021
@marcinwrobel1986 marcinwrobel1986 marked this pull request as draft July 15, 2021 15:23
@marcinwrobel1986 marcinwrobel1986 marked this pull request as ready for review July 19, 2021 07:48
@marcinwrobel1986
Copy link
Collaborator Author

@bartlettroscoe Hello Ross, I think it's ready to be reviewed

@bartlettroscoe
Copy link
Member

@bartlettroscoe Hello Ross, I think it's ready to be reviewed

Thanks @marcinwrobel1986! I will take care of this after #389 (which will test this automatically :-))

@bartlettroscoe bartlettroscoe added this to In Progress in TriBITS Refactor via automation Jul 21, 2021
@bartlettroscoe bartlettroscoe force-pushed the 377-provide-feedback-on-TriBITS-package-dependencies-data-structures branch from a9b578d to b03d7a2 Compare August 12, 2021 14:14
@bartlettroscoe
Rename tribits_save_off_dependencies_vars() to tribits_save_off_depen…
00ef148 
…dency_vars() (TriBITSPub#63)

This name is more consistent with related macro names.
bartlettroscoe
bartlettroscoe approved these changes Aug 12, 2021
View reviewed changes
Copy link
Member

@bartlettroscoe bartlettroscoe left a comment

Choose a reason for hiding this comment

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

I suspect this may conflict with PR #403 but I will take care of those conflicts after this merges.

I really need to run the spell checker on this code base more often to catch things like this. (But I am not sure it would catch misspellings inside of longer macro/function names like this.) I guess I just need to be more careful when first writing these to separate out the words and spell check them individually and then concatenate them into the macro/function name.

@marcinwrobel1986
Copy link
Collaborator Author

I suspect this may conflict with PR #403 but I will take care of those conflicts after this merges.

Thanks Ross!

I really need to run the spell checker on this code base more often to catch things like this. (But I am not sure it would catch misspellings inside of longer macro/function names like this.) I guess I just need to be more careful when first writing these to separate out the words and spell check them individually and then concatenate them into the macro/function name.

An IDE with spell check wouldn't be a bad choice :)

@bartlettroscoe
Copy link
Member

An IDE with spell check wouldn't be a bad choice :)

Yea, it may be time to look into that, even if it is just inside of emacs. (Getting an IDE to work on all of the various SNL machines is very difficult.)

@bartlettroscoe bartlettroscoe merged commit 76f3afa into TriBITSPub:master Aug 12, 2021
TriBITS Refactor automation moved this from In Progress to Done Aug 12, 2021
@bartlettroscoe
Copy link
Member

An IDE with spell check wouldn't be a bad choice :)

@marcinwrobel1986, actually, emacs has a built-in spell checker highlighter with the flyspell mode and it actually shows you misspellings of individual words in longer identifiers like tribits_read_defined_external_and_intenral_toplevel_packages_lists(). For example:

image

As you can see, it shows words that are common to the language or the code-base that are not in the dictionary like CMAKE, CMakeCache.txt. I find this highlighting of source code annoying which is why I usually don't turn it on and I have not found a easy way to set language or directory/specific dictionaries to address these correct common words so the incorrect words stand out.

@marcinwrobel1986
Copy link
Collaborator Author

Hello Ross, I don't use emacs, but I have found something. Maybe it's worth to give it a try:
Spell Checking in Emacs

bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this pull request Sep 4, 2021
@bartlettroscoe
Merge branch 'tribits_github_snapshot' into tribits-snapshot-2021-09-03
3c77011 
Brings in numerous refactorings to TriBITS over the last 3 months, but there
should be no breaks in backward compatibility.  Almost every file in TriBITS
is changed due to the lower-casing of command, macro and function names in PR
TriBITSPub/TriBITS#379.  But the main driver for this snapshot is to bring in
the change in PR TriBITSPub/TriBITS#413 that should make it so that Kokkos
INTERFACE_COMPILE_OPTIONS get propagated to downstream targets in TriBITS and
therefore to external customers through installed <Package>Config.cmake files
and IMPORTED targets.  I should have done several snapshots in the last few
months and not done a big snapshot like this (but I have been testing with
Trilinos locally along the way).

Overall, this merge brings in changes from a bunch of TriBITS PRs including
(from most recent):

* TriBITSPub/TriBITS#413: Change internal TriBITS target_link_libraries() to
  PUBLIC (TriBITSPub/TriBITS#299) component: core type: enhancement

* TriBITSPub/TriBITS#410: Upgrade from cmake 3.21.0 to 3.21.2
  (TriBITSPub/TriBITS#363, TriBITSPub/TriBITS#394)

* TriBITSPub/TriBITS#394: DO NOT MERGE: Show TriBITS test failures with CMake
  3.21.0 that don't occur with CMake 3.17.5 (TriBITSPub/TriBITS#363)

* TriBITSPub/TriBITS#409: Add getTestDictStatusField() to handle empty
  'status' field (SESW-383) component: ci_support type: enhancement

* TriBITSPub/TriBITS#408: Deal with spaces in CDash url parts (SESW-383)
  component: ci_support type: enhancement

* TriBITSPub/TriBITS#403: Spelling fixes

* TriBITSPub/TriBITS#407: Move tribits_get_build_url_and_write_to_file() to
  stand-alone module (TriBITSPub/TriBITS#154) component: ctest_driver type:
  enhancement

* TriBITSPub/TriBITS#388: Fixing typos (TriBITSPub/TriBITS#377)

* TriBITSPub/TriBITS#406: Fix tribits_ctest_driver() package-by-package mode
  for CMake 3.19+ (TriBITSPub/TriBITS#363, TriBITSPub/TriBITS#394) component:
  ctest_driver type: bug

* TriBITSPub/TriBITS#401: Improve GitHub Actions and CDash integration
  (TriBITSPub/TriBITS#154) type: enhancement

* TriBITSPub/TriBITS#366: CI: draft action yaml

* TriBITSPub/TriBITS#402: Revert some incorrect uppercase->lowercase changes

* TriBITSPub/TriBITS#387: Build and deploy TriBITS documentation with Sphinx
  (TriBITSPub/TriBITS#386) component: documentation type: enhancement

* TriBITSPub/TriBITS#398: Deal with pr null in not preprending build name
  (TriBITSPub/TriBITS#363) type: bug

* TriBITSPub/TriBITS#396: Send PR results to 'Pull Request' CDash group and
  add PR ID to build names (TriBITSPub/TriBITS#363) type: enhancement

* TriBITSPub/TriBITS#397: Print the ninja path and version
  (TriBITSPub/TriBITS#363)

* TriBITSPub/TriBITS#393: GitHub Actions based testing for TriBITS
  (TriBITSPub/TriBITS#363) type: enhancement

* TriBITSPub/TriBITS#389: TriBITS CI testing with GitHub Actions
  (TriBITSPub/TriBITS#363) type: enhancement

* TriBITSPub/TriBITS#392: Fix broken tests for non-Fortran and CMake 3.21
  builds (TriBITSPub/TriBITS#363) component: core

* TriBITSPub/TriBITS#391: Fix up build_docs.sh for Sphinx doc generation
  (TriBITSPub/TriBITS#386) component: documentation type: enhancement

* TriBITSPub/TriBITS#390: Add test for doc generation and fix usage of Python3
  component: documentation type: bug

* TriBITSPub/TriBITS#385: Replace last few references to
  TribitsDevelopersGuide.html (TriBITSPub/TriBITS#381) component:
  documentation type: enhancement

* TriBITSPub/TriBITS#384: Split TribitsDevelopersGuide.* into
  TribitsUsersGuide.* and TribitsMaintainersGuide.* (TriBITSPub/TriBITS#381)
  component: documentation type: enhancement

* TriBITSPub/TriBITS#383: Remove endfunction(<string>) and endmacro(<string>)
  (TriBITSPub/TriBITS#274, TriBITSPub/TriBITS#382) component: common_tpls
  type: bug

* TriBITSPub/TriBITS#380: More package-arch data-structure documentation
  updates (TriBITSPub/TriBITS#63) component: documentation type: enhancement

* TriBITSPub/TriBITS#379: Convert TriBITS to lower-case CMake command, macro,
  and function names (TriBITSPub/TriBITS#274)

The following files were conflicting where I went with what is on the Trilinos
'develop' branch:

* cmake/tribits/common_tpls/FindTPLBLAS.cmake
* cmake/tribits/common_tpls/FindTPLLAPACK.cmake
* cmake/tribits/common_tpls/FindTPLNetcdf.cmake

(It looks like the above changes never made it back into TriBITS proper.  The
conflicts were due to the case changes in cmake command calls in these files
due to TriBITSPub/TriBITS#379.)

There was also a conflict in the file:

* cmake/tribits/core/installation/TribitsProjectConfigTemplate.cmake.in

I looked at that one carefully and I think that may have been due to fixes on
both sides and then the case changes from TriBITSPub/TriBITS#379.
@bartlettroscoe bartlettroscoe mentioned this pull request Sep 4, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bartlettroscoe bartlettroscoe bartlettroscoe approved these changes

Assignees

@marcinwrobel1986 marcinwrobel1986

Labels
None yet
Projects
TriBITS Refactor
Status: Done
TriBITS Refactor
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@marcinwrobel1986 @bartlettroscoe