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

Add safety feature for testing algo changes #35

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

danholdaway
Copy link
Collaborator

When the JCB tests are being run (from anywhere) the code will clone the main JCB repo and when doing so it will look for the branch name of the PR. Then it will run a script to clone all the clients that are registered for testing as well as the algorithms repo. This script will use the branch name of the main JCB repo and if found for any clients clone that branch.

Now imagine this scenario: someone is making changes to their client (say gdas) and simultaneously making changes to jcb-algorithms. They might realize that they need to change another client to accommodate the algorithm changes so they make a branch with the same name in (say) rrfs. However if they do not have that branch in the main jcb repo there wouldn't exist a mechanism to check out the correct branch of jcb-algorithms and the other clients during the integration tests. We could put that kind of logic in the GitHub script but that would be complicated and have to propagate to every jcb repo.

The change in this code forces the user to make a branch of the main jcb repo if they have created the same branch in the jcb-algorithms branch and thus provide all the mechanisms for the scripts to check everything out properly.

The algorithms should really be part of JCB since they are generic and it would avoid the need to do this. But because of EE2 compliance we can't have YAMLs with source code, which is why they are in a separate repo.

Copy link
Contributor

@CoryMartin-NOAA CoryMartin-NOAA left a comment

Choose a reason for hiding this comment

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

Can we make this only do this for certain things? As I understand it, if we turn off channel 1 of ATMS, wouldn't that require a corresponding jcb-algorithms branch/PR? Seems like that is unnecessary. Can we limit it to just if certain files are modified in the PR?

EDIT: Nevermind, I can't read :-)

@danholdaway danholdaway merged commit 3cd697f into develop Oct 23, 2024
1 check passed
@danholdaway danholdaway deleted the feature/testing_safety_feature branch October 23, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants