-
Notifications
You must be signed in to change notification settings - Fork 559
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
Remove old framework/CI settings and infrastructure #10648
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@e10harvey, three of these files being deleted are for an old SEMS-based configuration of Trilinos that is not related to the Trilinos Framework's PR testing system. These files are:
- cmake/std/sems/SEMSDevEnv.cmake
- cmake/std/sems/checkin-test-sems.sh
- cmake/std/sems/get_default_modules.sh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes from my last review are shown here. It shows the 4 files added back:
- cmake/std/sems/SEMSDevEnv.cmake
- cmake/std/sems/checkin-test-sems.sh
- cmake/std/sems/get_default_modules.sh
- cmake/std/sems/trilinos_jsrun
@e10harvey, a potentially better way to add back these 4 files is to create a single commit to do it and make it clear what you are doing. For example, you can do this:
$ cd Trilinos/
$ git checkout TRILFRAME-312
$ git reset --hard baebf2d26708c44a5a9112205a490b889034c381
$ git checkout e2d0f8fba270e2a1eb4ed045fe8074a4b939e6b8^ -- \
cmake/std/sems/SEMSDevEnv.cmake \
cmake/std/sems/checkin-test-sems.sh \
cmake/std/sems/get_default_modules.sh \
cmake/std/sems/trilinos_jsrun
$ git commit -a -m "Bring back files not unique to old Trilinos PR testing"
$ git push -f
Just a suggestion for achieving a cleaner and more obvious Git History.
(Note, I believe that it is usually better to create new commits to address reviewer comments than to redo the old commits and force push. That provides an easy-to-follow history for the development and review of a PR branch.)
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-10.1.243
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Using Repos:
Pull Request Author: e10harvey |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs 30 Mins. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-10.1.243
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
|
FYI: The above PR iteration failed due to 3318 mass random failures for the cuda build on vortex shown here showing:
which was reported many years ago in #6861 and one test failed as shown here showing:
which was reported years ago in #7122. I will reopen those issues. The machine 'vortex' is not a good machine to drive PR testing due to these random jsrun failures on this machine (which the vender has never been able to fix). Now all of the PR builds have to be started over again from scratch (because rebuilds was disabled recently) :-( |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-10.1.243
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
Using Repos:
Pull Request Author: e10harvey |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_PR_gcc-8.3.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-serial
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_gcc-7.2.0-debug
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_intel-17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_clang-10.0.0
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_cuda-10.1.243
Jenkins Parameters
Build InformationTest Name: Trilinos_PR_python3
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ bartlettroscoe ]! |
Status Flag 'Pull Request AutoTester' - AutoMerge IS ENABLED, but the Label AT: AUTOMERGE is not set. Either set Label AT: AUTOMERGE or manually merge the PR... |
@ZUUL42, @jwillenbring, @fryeguy52: Since CI testing has passed, I will merge this later today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to remove these now obsolete files. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good to see a lot of cleanup.
@trilinos/framework
Motivation
Now that GenConfig has been deployed, we can remove the old cmake and environment settings as well as their associated driver scripts.
Stakeholder Feedback
Testing
This was tested manually via pipeline replays that ran off of both the source and target branches:
Source branch results.
Target branch results.
This PR resolves TRILFRAME-312: Remove old pre-GenConfig infrastructure.
Reviewers: Please verify that this old infrastructure is not used outside of MasterMerge and PullRequest testing. If this infrastructure is in use outside of MasterMerge and PullRequest testing, we should consider disabling that or migrating it to GenConfig before this merges.
CC: @srbdev