-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat : Detect ibm_catalog.json #1067
Conversation
@arya-girish-k This change looks good to me. I'm happy to merge if you want to mark the PR as ready for review? |
module-assets/ci/license_checker.sh
Outdated
|
||
# ensure LICENSE file exists if .tf file is detected incd root directory | ||
|
||
count=$(find . -maxdepth 1 \( -name "*.tf" -o -name "ibm_catalog.json" \) 2>/dev/null | wc -l | xargs) |
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.
@ocofaigh Do we want to set maxdepth if looking for all .tf files?
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.
This is a valid point. I ran this command against the terraform-ibm-common-utilities repo, and indeed it returned the count as 0 since the terraform code is not in the root of the repo.
However if we remove it I notice that it will pick up tf files from the common-dev-assets git submodule. e.g:
% find . \( -name "*.tf" -o -name "ibm_catalog.json" \) 2>/dev/null
./examples/crn-parser/outputs.tf
./examples/crn-parser/main.tf
./examples/crn-parser/variables.tf
./examples/crn-parser/provider.tf
./examples/crn-parser/version.tf
./modules/crn-parser/outputs.tf
./modules/crn-parser/main.tf
./modules/crn-parser/variables.tf
./modules/crn-parser/version.tf
./common-dev-assets/examples/mock_tf_code/outputs.tf
./common-dev-assets/examples/mock_tf_code/main.tf
./common-dev-assets/examples/mock_tf_code/variables.tf
./common-dev-assets/examples/mock_tf_code/provider.tf
./common-dev-assets/examples/mock_tf_code/version.tf
@arya-girish-k can we update the logic to not use maxdepth but also to ignore git submodules?
@ocofaigh ,I marked as ready for review. |
cc:@SarikaSinha |
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.
module-assets/ci/license_checker.sh
Outdated
|
||
# ensure LICENSE file exists if .tf file or ibm_catalog.json is detected in root directory | ||
|
||
var1=$(find ./*.tf 2>/dev/null | wc -l | xargs) |
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.
This solution won't work - we need to scan the full repo for .tf files. Some repos (such as terraform-ibm-observability-da) will not have .tf files in the root, but will have them in other folders.
So what we need is to scan the full repo for .tf file, but ignore them if they are in a folder called common-dev-assets
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.
@ocofaigh ,Ok I will make the changes and update.
Description
This PR enhances the existing shell script by adding functionality to detect ibm_catalog.json files in a specified directory.After the changes the hook will run if ibm_catalog.json or .tf file is in the repo and if it exist it should mandate LICENSE file.
Issue:https://github.ibm.com/GoldenEye/issues/issues/8715
Release required?
x.x.X
)0.1.0
)X.x.x
)Release notes content
Run the pipeline
If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.
Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:
Checklist for reviewers
For mergers