Skip to content

Conversation

@EyalDelarea
Copy link
Contributor

@EyalDelarea EyalDelarea commented Jul 23, 2023

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • This pull request is on the dev branch.
  • I used gofmt for formatting the code before submitting the pull request.

Refactor scan-pull-request command to accept PR ID as input instead of relying on context.

  • Deleted all the git test folders for scan pull requests tests as we are downloading only specific branches.
  • Refactored scan-pull-requests command to use the singular scan-pull-request
  • Update Github templates

This will allow to use scan pull requests command with Jenkins webhook trigger, providing the pull request ID.

Jenkins Webhook Docs Update PR:
#399

Update:
When pull request origin from a forked repository, we need to know the owner in order to access the source code.
jfrog/froggit-go#102

@EyalDelarea EyalDelarea added safe to test Approve running integration tests on a pull request improvement Automatically generated release notes labels Jul 23, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Jul 23, 2023
@EyalDelarea EyalDelarea marked this pull request as ready for review July 23, 2023 15:07
@EyalDelarea EyalDelarea requested a review from sverdlov93 July 23, 2023 15:08
@EyalDelarea EyalDelarea added the safe to test Approve running integration tests on a pull request label Jul 24, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Jul 24, 2023
@EyalDelarea EyalDelarea added the safe to test Approve running integration tests on a pull request label Jul 24, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Jul 24, 2023
Copy link
Contributor

@sverdlov93 sverdlov93 left a comment

Choose a reason for hiding this comment

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

not finished the review yet

if g.PullRequestID, err = strconv.Atoi(pullRequestIDString); err != nil {
return err
if g.PullRequestID == UndefinedPrID {
if idStr := getTrimmedEnv(GitPullRequestIDEnv); idStr != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

why PullRequestID can't be string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The API accepts ID as an int ( froggit-go )

@EyalDelarea EyalDelarea added the safe to test Approve running integration tests on a pull request label Jul 26, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Jul 26, 2023
var vulnerabilitiesRows []formats.VulnerabilityOrViolationRow
var iacRows []formats.IacSecretsRow
targetBranch := repoConfig.Branches[0]
targetBranch := pullRequestDetails.Target.Name
Copy link
Contributor

Choose a reason for hiding this comment

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

pullRequestDetails.Target can't be nil?
pullRequestDetails.Source cant be nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They can be empty with default values, but they cannot be nil.

@github-actions
Copy link
Contributor

@EyalDelarea EyalDelarea added the safe to test Approve running integration tests on a pull request label Jul 26, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Jul 26, 2023
@github-actions
Copy link
Contributor

@EyalDelarea EyalDelarea temporarily deployed to frogbot July 26, 2023 12:22 — with GitHub Actions Inactive
@EyalDelarea EyalDelarea added the safe to test Approve running integration tests on a pull request label Jul 26, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Jul 26, 2023
@github-actions
Copy link
Contributor

@EyalDelarea EyalDelarea temporarily deployed to frogbot July 26, 2023 12:48 — with GitHub Actions Inactive
@EyalDelarea EyalDelarea added the safe to test Approve running integration tests on a pull request label Jul 26, 2023
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Jul 26, 2023
@github-actions
Copy link
Contributor

1 similar comment
@github-actions
Copy link
Contributor

@EyalDelarea EyalDelarea merged commit 25d5521 into jfrog:dev Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Automatically generated release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants