-
Notifications
You must be signed in to change notification settings - Fork 294
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(scanner): Adds branch name to FossID scan code. #8641
base: main
Are you sure you want to change the base?
feat(scanner): Adds branch name to FossID scan code. #8641
Conversation
efc3d6c
to
088c960
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8641 +/- ##
============================================
- Coverage 67.92% 67.88% -0.04%
- Complexity 1005 1165 +160
============================================
Files 244 244
Lines 7772 7732 -40
Branches 876 865 -11
============================================
- Hits 5279 5249 -30
- Misses 2110 2124 +14
+ Partials 383 359 -24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
651c2b3
to
bbf6e42
Compare
Commit messages should be written in "imperative mood" and titles should not end in a dot, so use "feat(scanner): Add a branch name to the FossID scan code" |
plugins/scanners/fossid/src/main/kotlin/FossIdNamingProvider.kt
Outdated
Show resolved
Hide resolved
Please also document the new builtin: https://github.com/oss-review-toolkit/ort/blob/main/plugins/scanners/fossid/src/main/kotlin/FossIdNamingProvider.kt#L28-L39 |
82595dc
to
4f493bc
Compare
35c3ced
to
c2f0f80
Compare
plugins/scanners/fossid/src/main/kotlin/FossIdNamingProvider.kt
Outdated
Show resolved
Hide resolved
plugins/scanners/fossid/src/main/kotlin/FossIdNamingProvider.kt
Outdated
Show resolved
Hide resolved
plugins/scanners/fossid/src/main/kotlin/FossIdNamingProvider.kt
Outdated
Show resolved
Hide resolved
plugins/scanners/fossid/src/test/kotlin/FossIdNamingProviderTest.kt
Outdated
Show resolved
Hide resolved
plugins/scanners/fossid/src/test/kotlin/FossIdNamingProviderTest.kt
Outdated
Show resolved
Hide resolved
Add branch name (revision) to FossID scan code for easier identification of scanned source code. As FossID accepts maximum of 255 characters in scan code, branch name is trimmed to size, that is compliant with this constraint. Signed-off-by: Kamil Bielecki <kamil.bielecki@pl.bosch.com>
c2f0f80
to
f1628a0
Compare
} | ||
|
||
/** | ||
* Replaces non-standard characters in branch name and trimming it's length to one that will not exceed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and trims its ...
|
||
/** | ||
* Replaces non-standard characters in branch name and trimming it's length to one that will not exceed | ||
* maximum length of FossID scan ID, when combined with rest of variables |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the rest of variables
namingConventionVariables | ||
).length | ||
|
||
val maxBranchNameLength = MAX_SCAN_CODE_LEN - noBranchScanCodeLength |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this code, what happens if someone put a pattern in namingScanPattern
which leads to a noBranchScanCodeLength
> 254 ?
@@ -1,5 +1,5 @@ | |||
/* | |||
* Copyright (C) 2021 The ORT Project Authors (see <https://github.com/oss-review-toolkit/ort/blob/main/NOTICE>) | |||
* Copyright (C) 2024 The ORT Project Authors (see <https://github.com/oss-review-toolkit/ort/blob/main/NOTICE>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The year hear is when the file was created originally; so this should stay 2021. But the copyright year in the test file you have newly added should be 2024.
val expectedTimestamp = "20240401_100000" | ||
|
||
"create code without branch name, when it's empty" { | ||
mockkStatic(LocalDateTime::class) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using mockStatic
, it is required to reset mocking afterward to prevent that other tests are affected. This is easily achieved by adding an afterSpec
method that contains an unmockAll()
call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably using afterEach
is yet safer, to prevent unintended side-effects between individual test cases.
Adds branch name (revision) to FossID scan code for easier identification of scanned source code.
As FossID accepts maximum of 255 characters in scan code, branch name is trimmed to size, that is compliant with this constraint.