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

feat!: remove feature cim-field-report generation #885

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

dvarasani-crest
Copy link
Contributor

This PR removes the feature of generating cim-field-report.
ref: ADDON-73385

NOTE:

  • moved unit test file (test_report.py) of cim-compliance report generation test from test_tools folder to test_utilities

@dvarasani-crest dvarasani-crest requested a review from a team as a code owner August 21, 2024 06:19
@kdoroszko-splunk
Copy link
Contributor

kdoroszko-splunk commented Aug 21, 2024

What is the reason of moving from tests/unit/tests_standard_lib/test_tools/test_cim_report.py to tests/unit/tests_standard_lib/test_utilities/test_cim_report.py? I can see that tests structure mimics structure of submodules: https://github.com/splunk/pytest-splunk-addon/tree/main/pytest_splunk_addon.

@dvarasani-crest
Copy link
Contributor Author

@kdoroszko-splunk I have moved that test file to actually align that with the submodules. test_report.py tests for cim-report generation which is for function generate_report of standard_lib/utilities/junit_parser.py::generate_report().

@kdoroszko-splunk
Copy link
Contributor

@kdoroszko-splunk I have moved that test file to actually align that with the submodules. test_report.py tests for cim-report generation which is for function generate_report of standard_lib/utilities/junit_parser.py::generate_report().

Right, thanks for clarifying.

@mkolasinski-splunk
Copy link
Contributor

mkolasinski-splunk commented Aug 21, 2024

@dvarasani-crest is ! sign in PR title intentional?
Consider changing base branch to release/v5.4.0

Beside above, LGTM!

@dvarasani-crest
Copy link
Contributor Author

@dvarasani-crest is ! sign in PR title intentional? Consider changing base branch to release/v5.4.0

Beside above, LGTM!

@mkolasinski-splunk I have added ! to indicate this PR as a breaking change. Also, since this PR contains a BREAKING change commit, do you still think we need to rebase it to release/5.4.0? because merging this commit to release/5.4.0 would lead to release v6.0.0 when we merge release/5.4.0 to main.
Let me know if we do not want to consider this as a BREAKING change.

@mkolasinski-splunk
Copy link
Contributor

@dvarasani-crest is ! sign in PR title intentional? Consider changing base branch to release/v5.4.0
Beside above, LGTM!

@mkolasinski-splunk I have added ! to indicate this PR as a breaking change. Also, since this PR contains a BREAKING change commit, do you still think we need to rebase it to release/5.4.0? because merging this commit to release/5.4.0 would lead to release v6.0.0 when we merge release/5.4.0 to main. Let me know if we do not want to consider this as a BREAKING change.

You are right - let's keep it this way. Regarding base branch - I think we can merge it to develop as there are being aggregated changes for v6.0.0 release as far as I can see: #871

@dvarasani-crest dvarasani-crest changed the base branch from main to develop August 22, 2024 07:19
@dvarasani-crest
Copy link
Contributor Author

@dvarasani-crest is ! sign in PR title intentional? Consider changing base branch to release/v5.4.0
Beside above, LGTM!

@mkolasinski-splunk I have added ! to indicate this PR as a breaking change. Also, since this PR contains a BREAKING change commit, do you still think we need to rebase it to release/5.4.0? because merging this commit to release/5.4.0 would lead to release v6.0.0 when we merge release/5.4.0 to main. Let me know if we do not want to consider this as a BREAKING change.

You are right - let's keep it this way. Regarding base branch - I think we can merge it to develop as there are being aggregated changes for v6.0.0 release as far as I can see: #871

Updated base branch to develop.

@dvarasani-crest dvarasani-crest merged commit c4f72fe into develop Aug 22, 2024
22 of 23 checks passed
@dvarasani-crest dvarasani-crest deleted the remove-cim-field-report branch August 22, 2024 09:14
@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2024
@srv-rr-github-token
Copy link
Contributor

🎉 This issue has been resolved in version 5.4.0-beta.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants