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

Improved tech detection when tech is provided by user #175

Conversation

eranturgeman
Copy link
Contributor

@eranturgeman eranturgeman commented Sep 11, 2024

  • The pull request is targeting the dev branch.
  • The code has been validated to compile successfully by running go vet ./....
  • The code has been formatted properly using go fmt ./....
  • All static analysis checks passed.
  • All tests have passed. If this feature is not already covered by the tests, new tests have been added.
  • All changes are detailed at the description. if not already covered at JFrog Documentation, new documentation have been added.

This PR addresses an issue with incorrect technology detection in Yarn projects, particularly when there are multiple sub-projects that only contain a Yarn descriptor without any specific Yarn indicators. In a multi-project (but not multi-module) Yarn environment, if the inner projects only include the package.json file without any of Yarn’s standard indicators (.yarnrc.yml, yarn.lock, .yarn, “yarnrc”), the auto-detection mechanism incorrectly identifies these as NPM projects.

Since package.json cannot be used as an indicator for both NPM and Yarn to avoid conflicts, I implemented the following logic:
If the user specifies a particular technology (via command flags or the install command [Frogbot]), we take that into account when verifying the technology. This allows us to verify the specified technology using both its descriptors and indicators and not just its indicators.

As a result, when a ‘yarn install’ command is provided for a project and a package.json file is present, we can confidently determine that Yarn is being used, not NPM.

Another test case needs to be added to TestGetTechInformationFromWorkingDir. This test case discovered an issue that is fixed this PR: #179
The test case will be added along with the fix.

…n tech by descriptor and not just indicator in case the tech is provided by the user
…o force-tech-detection-if-provided-from-install-command
@eranturgeman eranturgeman added improvement Automatically generated release notes safe to test Approve running integration tests on a pull request labels Sep 11, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 11, 2024
…o force-tech-detection-if-provided-from-install-command
@eranturgeman eranturgeman added the safe to test Approve running integration tests on a pull request label Sep 11, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 11, 2024
…o force-tech-detection-if-provided-from-install-command
@eranturgeman eranturgeman added the safe to test Approve running integration tests on a pull request label Sep 15, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 15, 2024
Copy link
Contributor

@attiasas attiasas left a comment

Choose a reason for hiding this comment

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

Nice!, take a look at my comments.

utils/techutils/techutils.go Outdated Show resolved Hide resolved
utils/techutils/techutils_test.go Outdated Show resolved Hide resolved
@eranturgeman eranturgeman changed the title Enable tech detection by 'install' command Improved tech detection when tech is provided by user Sep 15, 2024
…o force-tech-detection-if-provided-from-install-command
@eranturgeman eranturgeman added the safe to test Approve running integration tests on a pull request label Sep 15, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 15, 2024
…o force-tech-detection-if-provided-from-install-command
@eranturgeman eranturgeman added the safe to test Approve running integration tests on a pull request label Sep 15, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Sep 15, 2024
Copy link

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


@eranturgeman eranturgeman merged commit f6f8065 into jfrog:dev Sep 15, 2024
9 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