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: added support for gradle #99

Merged
merged 7 commits into from
Apr 4, 2024
Merged

Conversation

olavtar
Copy link
Collaborator

@olavtar olavtar commented Mar 26, 2024

Description

Added support for gradle

Related issue (if any): fixes #issue_number_goes_here

Checklist

  • I have followed this repository's contributing guidelines.
  • I will adhere to the project's code of conduct.

Additional information

Anything else?

Copy link
Collaborator

@zvigrinberg zvigrinberg left a comment

Choose a reason for hiding this comment

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

Hi @olavtar
Please look at the Comments.
the mocks stubbing in the tests should be adjusted in order that the tests will be successful
also there are some minor changes in the code.
In addition, i saw also problem with the comparison between expected and actual stack analysis sbom - https://github.com/RHEcosystemAppEng/exhort-java-api/actions/runs/8441141382/job/23119617921#step:8:747, please check to reveal the cause for that.
anyway, good job!.

@zvigrinberg
Copy link
Collaborator

zvigrinberg commented Mar 28, 2024

As discussed and agreed, //exhortignore comment as suffix of the line containing dependency , will mark the artifact for exclusion from analysis.

Copy link
Collaborator

@zvigrinberg zvigrinberg left a comment

Choose a reason for hiding this comment

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

Hi @olavtar , So it's not only the expiration of the exhort backend that failed the Integration tests, you should also create a minimal simple build.gradle file (maximum 2-3 dependencies), and put it in a new directory gradle in src/test/resources/tst_manifests/it/gradle , like all other package managers. if you need to add properties file, also add it, and also load this file to the IT test only if it's gradle package manager. please try to do the build.gradle with minimum number of dependencies and without any other files dependencies ( maybe only properties file if you have to), Thanks.

This will prevent the nullPointerException that happening in IT Tests.

Signed-off-by: Olga Lavtar <olavtar@redhat.com>
Signed-off-by: Olga Lavtar <olavtar@redhat.com>
Signed-off-by: Olga Lavtar <olavtar@redhat.com>
Signed-off-by: Olga Lavtar <olavtar@redhat.com>
Signed-off-by: Olga Lavtar <olavtar@redhat.com>
Signed-off-by: Olga Lavtar <olavtar@redhat.com>
Copy link
Collaborator

@zvigrinberg zvigrinberg left a comment

Choose a reason for hiding this comment

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

@olavtar
Looks pretty good. ( now all tests passing including IT )
Just relate to my 2 minor comments.
and i didn't see documentation of gradle in README.md ( configuration, exhortignore for gradle with example, and etc.)

Signed-off-by: Olga Lavtar <olavtar@redhat.com>
Copy link
Collaborator

@zvigrinberg zvigrinberg left a comment

Choose a reason for hiding this comment

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

@olavtar LGTM, Approved.
Great Job! and thanks for your efforts.
you can merge whenever you ready.

Zvi.

@olavtar olavtar merged commit 6563be7 into RHEcosystemAppEng:main Apr 4, 2024
5 checks passed
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