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

Scan PR - Add option to scan source and most common ancestor target c… #763

Merged
merged 6 commits into from
Oct 30, 2024

Conversation

attiasas
Copy link
Contributor

@attiasas attiasas commented Oct 8, 2024

  • 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.
  • Update documentation about new features / new supported technologies

Scan PR/MR command

We are scanning both the source and target branches.
Then we compare the results and filter only those found in the source, not in the target.

The new option calculates the most common ancestor commit by running git merge-base source target, checking out to the commit, and treating it as the target to be merged to.
To use this option you can:

  • Specify it as an env var: JF_USE_MOST_COMMON_ANCESTOR_AS_TARGET
  • Specify it in Frogbot config: useMostCommonAncestorAsTarget (at Git section)

@attiasas attiasas added safe to test Approve running integration tests on a pull request improvement Automatically generated release notes labels Oct 8, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Oct 8, 2024
@attiasas attiasas marked this pull request as ready for review October 9, 2024 06:37
@attiasas attiasas added the safe to test Approve running integration tests on a pull request label Oct 9, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Oct 9, 2024
@eranturgeman
Copy link
Contributor

fix title + consider adding an explanation about when one should use this feature

utils/git.go Outdated Show resolved Hide resolved
utils/git.go Outdated Show resolved Hide resolved
@eranturgeman
Copy link
Contributor

Where does useMostCommonAncestorAsTarget field be inserted in frogbot-config.yml? is it part of the 'Project' list or a general definition.
We need to consider if we even want to add it as a field to frogbot-config.yml since the config file is going to change soon and maybe we should avoid adding new field to it

In addition, please open a ticket to add this new field to ConfigProfile

Copy link
Contributor

@eranturgeman eranturgeman left a comment

Choose a reason for hiding this comment

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

Looks good, see my comments

@attiasas attiasas added the safe to test Approve running integration tests on a pull request label Oct 30, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Oct 30, 2024
Copy link
Contributor

👍 Frogbot scanned this pull request and did not find any new security issues.


@attiasas attiasas merged commit d1c5866 into jfrog:dev Oct 30, 2024
33 checks passed
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