Skip to content

Conversation

camilamacedo86
Copy link
Collaborator

No description provided.

@coveralls
Copy link

coveralls commented Aug 25, 2021

Pull Request Test Coverage Report for Build 1167479480

  • 0 of 3 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.001%) to 0.857%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/reports/custom/deprecated_api_report.go 0 3 0.0%
Totals Coverage Status
Change from base Build 1167419260: -0.001%
Covered Lines: 18
Relevant Lines: 2101

💛 - Coveralls

generate-pkgs: install ## ensure that you have update the the date of the report.
go run ./hack/scripts/packages/generate.go --image=testdata/reports/redhat_certified_operator_index/bundles_registry.redhat.io_redhat_certified_operator_index_v4.9_2021-08-22.json
go run ./hack/scripts/packages/generate.go --image=testdata/reports/redhat_redhat_marketplace_index/bundles_registry.redhat.io_redhat_redhat_marketplace_index_v4.9_2021-08-22.json
go run ./hack/scripts/packages/generate.go --image=testdata/reports/redhat_certified_operator_index/bundles_registry.redhat.io_redhat_certified_operator_index_v4.8_2021-08-21.json
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should pass these in as env vars so it's not causing a git diff each time we want a new report target?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why should we inform the required input which changes for can each report generate via env var instead of use flags?
the audit tool is a CLI tool. It is using Cobra which allows us to easily create commands and have their flags. If you run --help you can check all optional flags and the values that you can inform.

The helper in the makefile needs to be updated with the JSON file every time that we add re-generate the reports. See the comment ensure that you have update the the date of the report. The Makefile here was added just to make it easier re-generate these reports and make clear/understandable how to do it for others in case I cannot do it and another person requires to run.

Also, see the comment description of the helper which appears via make --help This method should be removed soon. It is only an internal helper

@@ -0,0 +1,383 @@
##############################################
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be good if report data wasn't in a place that seems like it should have code hack/scripts/packages/ and it might be good to put it somewhere that is being gitignore'd -- it is just output of the tool, not source, right?

Copy link
Collaborator Author

@camilamacedo86 camilamacedo86 Aug 27, 2021

Choose a reason for hiding this comment

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

The scripts directory will be fully removed after 4.9-GA.
If you look it is all deprecated. Why store the reports?

  • Make easier: I do not need to re-gen that more than once when/if I need these data for the communications that we need to do before 4.9-GA.
  • Be able to check the previous reports if/when required.

# - The following results were obtained programmatically from:
# - Image name: registry.redhat.io/redhat/community-operator-index:v4.8
# - Image ID: sha256:62e01b2214adbbcbad086b8e1e580cd8a95da4f6c904abeebf923fd52eb077c1
# - Image Created at: 2021-08-20T21:38:31.852294899Z
Copy link
Collaborator

Choose a reason for hiding this comment

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

same comment about gitignore'd and new location preferable

@@ -0,0 +1,282 @@
##############################################
Copy link
Collaborator

Choose a reason for hiding this comment

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

same comment about gitignore'd and new location preferable

# - Image Created at: 2021-08-19T18:05:29.215979732Z
# - From JSON report generated at: 2021-08-21
##############################################
-------------------------------------
Copy link
Collaborator

Choose a reason for hiding this comment

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

same comment about gitignore'd and new location preferable

@camilamacedo86
Copy link
Collaborator Author

Hi @bentito,

As we spoke I am moving forward here, but we can do changes afterwords by sure.
Note that all this code and files will be removed in 2 months time.

Thank you for your time and review.

@camilamacedo86 camilamacedo86 merged commit c2b598f into operator-framework:main Aug 27, 2021
@camilamacedo86 camilamacedo86 deleted the text-helper-reports branch August 27, 2021 15:02
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.

4 participants