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

Passive voice in code comments #9123

Merged
merged 2 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion analyzer/src/main/kotlin/Analyzer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class Analyzer(private val config: AnalyzerConfiguration, private val labels: Ma
}.toMap(mutableMapOf())

// Check whether there are unmanaged files (because of deactivated, unsupported, or non-present package
// managers) which we need to attach to an artificial "unmanaged" project.
// managers) which need to get attached to an artificial "unmanaged" project.
val managedDirs = managedFiles.values.flatten().mapNotNull { it.parentFile }
val hasOnlyManagedDirs = absoluteProjectPath in managedDirs || absoluteProjectPath.walk().maxDepth(1)
.all { it.isDirectory && (it in managedDirs || it.name in VCS_DIRECTORIES) }
Expand Down
4 changes: 2 additions & 2 deletions analyzer/src/main/kotlin/AnalyzerResultBuilder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class AnalyzerResultBuilder {
fun addResult(projectAnalyzerResult: ProjectAnalyzerResult) =
apply {
// TODO: It might be, e.g. in the case of PIP "requirements.txt" projects, that different projects with
// the same ID exist. We need to decide how to handle that case.
// the same ID exist. Decide how to handle that case.
val existingProject = projects.find { it.id == projectAnalyzerResult.project.id }

if (existingProject != null) {
Expand Down Expand Up @@ -123,7 +123,7 @@ private fun AnalyzerResult.resolvePackageManagerDependencies(): AnalyzerResult {
}

// Package managers that do not use the dependency graph representation, might not have a check implemented to
// verify that packages exist for all dependencies, so we need to disable the reference check here.
// verify that packages exist for all dependencies, so the reference check needs to be disabled here.
type to builder.build(checkReferences = false)
}

Expand Down
2 changes: 1 addition & 1 deletion analyzer/src/main/kotlin/PackageManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ abstract class PackageManager(
val mergedVcs = normalizedVcsFromPackage.merge(fallbackVcs)
if (mergedVcs != normalizedVcsFromPackage) {
// ... but if indeed metadata was enriched, overwrite the URL with the one from the fallback VCS
// information to ensure we get the correct base URL if additional VCS information (like a revision
// information to ensure to get the correct base URL if additional VCS information (like a revision
// or path) has been split from the original URL.
return mergedVcs.copy(url = fallbackVcs.url)
}
Expand Down
4 changes: 2 additions & 2 deletions buildSrc/src/main/kotlin/ort-kotlin-conventions.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ kotlin.target.compilations.apply {

configurations.all {
resolutionStrategy {
// Ensure all JRuby versions match our version to avoid Psych YAML library issues.
// Ensure all transitive JRuby versions match ORT's version to avoid Psych YAML library issues.
force(libs.jruby)

// Ensure that all transitive versions of Kotlin libraries match our version of Kotlin.
// Ensure that all transitive versions of Kotlin libraries match ORT's version of Kotlin.
force("org.jetbrains.kotlin:kotlin-reflect:${libs.versions.kotlinPlugin.get()}")
}
}
Expand Down
7 changes: 4 additions & 3 deletions clients/fossid-webapp/src/main/kotlin/FossIdRestService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,10 @@ interface FossIdRestService {
boundType.rawClass
)
val map = JSON_MAPPER.readValue<Map<Any, Any>>(p, mapType)
// we keep only the values of the map: when the FossID functions which return a PolymorphicList
// return a map, this is always the list of elements grouped by id. Since the ids are also
// present in the elements themselves, we don't lose any information by discarding the keys.

// Only keep the map's values: If the FossID functions which return a PolymorphicList return a
// map, it always is the list of elements grouped by id. Since the ids are also present in the
// elements themselves, no information is lost by discarding the keys.
PolymorphicList(map.values.toList())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ class FossId2021dot2Test : StringSpec({
}

"Scan status can be queried (2021.2)" {
// because the service caches the version, we must recreate it
// Recreate the version as the service caches it.
service = FossIdServiceWithVersion.create(service)

service.checkScanStatus("", "", SCAN_CODE_2021_2).shouldNotBeNull().run {
checkResponse("get scan status")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ class FossId2023dot1Test : StringSpec({
}

"Delete scan response can be parsed (2023.1)" {
// because the service caches the version, we must recreate it
// Recreate the version as the service caches it.
service = FossIdServiceWithVersion.create(service)

service.deleteScan("", "", SCAN_CODE_2021_2).shouldNotBeNull().run {
checkResponse("delete scan")

Expand Down
9 changes: 4 additions & 5 deletions downloader/src/main/kotlin/Downloader.kt
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ class Downloader(private val config: DownloaderConfiguration) {
val vcsMark = TimeSource.Monotonic.markNow()

try {
// Cargo in general builds from source tarballs, so we prefer source artifacts over VCS, but still use VCS
// if no source artifact is given.
// Cargo in general builds from source tarballs, so prefer source artifacts over VCS, but still use VCS if
// no source artifact is given.
val isCargoPackageWithSourceArtifact = pkg.id.type == "Cargo" && pkg.sourceArtifact != RemoteArtifact.EMPTY

if (!isCargoPackageWithSourceArtifact) {
Expand Down Expand Up @@ -273,8 +273,7 @@ class Downloader(private val config: DownloaderConfiguration) {
val workingTree = try {
applicableVcs.download(pkg, outputDirectory, config.allowMovingRevisions, recursive)
} catch (e: DownloadException) {
// TODO: We should introduce something like a "strict" mode and only do these kind of fallbacks in
// non-strict mode.
// TODO: Introduce something like a "strict" mode and only do these kind of fallbacks in non-strict mode.
val vcsUrlNoCredentials = pkg.vcsProcessed.url.replaceCredentialsInUri()
if (vcsUrlNoCredentials != pkg.vcsProcessed.url) {
// Try once more with any username / password stripped from the URL.
Expand Down Expand Up @@ -393,7 +392,7 @@ class Downloader(private val config: DownloaderConfiguration) {
}

/**
* Consolidate [projects] based on their VcsInfo without taking the path into account. As we store VcsInfo per project
* Consolidate [projects] based on their VcsInfo without taking the path into account. As VcsInfo is stored per project
* but many project definition files actually reside in different subdirectories of the same VCS working tree, it does
* not make sense to download (and scan) all of them individually, not even if doing sparse checkouts. Return a map that
* associates packages for projects in distinct VCS working trees with all other projects from the same VCS working
Expand Down
8 changes: 4 additions & 4 deletions model/src/main/kotlin/DependencyGraph.kt
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,9 @@ class DependencyReference(
val dependencies: Set<DependencyReference> = emptySet(),

/**
* The type of linkage used for the referred package from its dependent package. As most of our supported
* The type of linkage used for the referred package from its dependent package. As most of ORT's supported
* package managers / languages only support dynamic linking or at least default to it, also use that as the
* default value here to not blow up our result files.
* default value here to not blow up ORT result files.
*/
@JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = PackageLinkageValueFilter::class)
val linkage: PackageLinkage = PackageLinkage.DYNAMIC,
Expand Down Expand Up @@ -367,9 +367,9 @@ data class DependencyGraphNode(
val fragment: Int = 0,

/**
* The type of linkage used for the referred package from its dependent package. As most of our supported
* The type of linkage used for the referred package from its dependent package. As most of ORT's supported
* package managers / languages only support dynamic linking or at least default to it, also use that as the
* default value here to not blow up our result files.
* default value here to not blow up ORT result files.
*/
@JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = PackageLinkageValueFilter::class)
val linkage: PackageLinkage = PackageLinkage.DYNAMIC,
Expand Down
4 changes: 2 additions & 2 deletions model/src/main/kotlin/Package.kt
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ import org.ossreviewtoolkit.utils.spdx.SpdxOperator
/**
* A generic descriptor for a software package. It contains all relevant metadata about a package like the name,
* version, and how to retrieve the package and its source code. It does not contain information about the package's
* dependencies, however. This is because at this stage we would only be able to get the declared dependencies, whereas
* we are interested in the resolved dependencies. Resolved dependencies might differ from declared dependencies due to
* dependencies, however. This is because at this stage ORT would only be able to get the declared dependencies, whereas
* the resolved dependencies are of interest. Resolved dependencies might differ from declared dependencies due to
* specified version ranges, or change depending on how the package is used in a project due to the build system's
* dependency resolution process. For example, if multiple versions of the same package are used in a project, the build
* system might decide to align on a single version of that package.
Expand Down
4 changes: 2 additions & 2 deletions model/src/main/kotlin/PackageReference.kt
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ data class PackageReference(
override val id: Identifier,

/**
* The type of linkage used for the referred package from its dependent package. As most of our supported
* The type of linkage used for the referred package from its dependent package. As most of ORT's supported
* package managers / languages only support dynamic linking or at least default to it, also use that as the
* default value here to not blow up our result files.
* default value here to not blow up ORT result files.
*/
@JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = PackageLinkageValueFilter::class)
override val linkage: PackageLinkage = PackageLinkage.DYNAMIC,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class Composer(
// required, but are not listed in composer.lock as installed.
// If we didn't handle them specifically, we would report them as missing when trying to load the
// dependency information for them. We can't simply put these "virtual" packages in the normal package
// map as this would cause us to report a package which is not actually installed with the contents of
// map as this would cause ORT to report a package which is not actually installed with the contents of
// the "replacing" package.
val virtualPackages = parseVirtualPackageNames(packages, projectPackageInfo, lockfile)

Expand Down
2 changes: 1 addition & 1 deletion plugins/package-managers/node/src/main/kotlin/Npm.kt
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ open class Npm(
val scopeNames = setOfNotNull(
// Optional dependencies are just like regular dependencies except that NPM ignores failures when
// installing them (see https://docs.npmjs.com/files/package.json#optionaldependencies), i.e. they are
// not a separate scope in our semantics.
// not a separate scope in ORT semantics.
buildDependencyGraphForScopes(
project,
workingDir,
Expand Down
16 changes: 8 additions & 8 deletions plugins/scanners/fossid/src/main/kotlin/FossId.kt
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ class FossId internal constructor(

// A list of all scans created in an ORT run, to be able to delete them in case of error.
// The reasoning is that either all these scans are successful, either none is created at all (clean slate).
// A use case is that an ORT run is created regularly e.g. nightly, and we want to have exactly the same amount
// of scans for each package.
// A use case is that an ORT run is created regularly, e.g. nightly, and exactly the same amount of scans for each
// package is wanted.
private val createdScans = mutableSetOf<String>()

private val service = runBlocking { FossIdRestService.create(config.serverUrl) }
Expand Down Expand Up @@ -433,8 +433,8 @@ class FossId internal constructor(
): List<Scan> {
val scans = filter {
val isArchived = it.isArchived == true
// The scans in the server contain the url with the credentials, so we have to remove it for the
// comparison. If we don't, the scans won't be matched if the password changes!
// The scans in the server contain the URL with credentials, so these have to be removed for the comparison.
// Otherwise scans would not be matched if the password changed.
val urlWithoutCredentials = it.gitRepoUrl?.replaceCredentialsInUri()
!isArchived && urlWithoutCredentials == url
}.sortedByDescending { it.id }
Expand Down Expand Up @@ -530,7 +530,7 @@ class FossId internal constructor(
val mappedUrl = urlProvider.getUrl(urlWithoutCredentials)
val mappedUrlWithoutCredentials = mappedUrl.replaceCredentialsInUri()

// we ignore the revision because we want to do a delta scan
// Ignore the revision for delta scans.
val recentScans = scans.recentScansForRepository(
mappedUrlWithoutCredentials,
projectRevision = projectRevision,
Expand Down Expand Up @@ -755,9 +755,9 @@ class FossId internal constructor(
DownloadStatus.FAILED -> error("Could not download scan: ${response.message}.")

else -> {
// There is a bug with the FossID server version < 20.2: Sometimes the download is complete, but it
// stays in state "NOT FINISHED". Therefore, we check the output of the Git fetch to find out
// whether the download is actually done.
// There is a bug in FossID server version < 20.2: Sometimes the download is complete, but it stays
// in "NOT FINISHED" state. Therefore, check the output of the Git fetch command to find out whether
// the download has actually finished.
val message = response.message
val currentVersion = checkNotNull(Semver.coerce(version))
val minVersion = checkNotNull(Semver.coerce("20.2"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const val MAX_SUPPORTED_OUTPUT_FORMAT_MAJOR_VERSION = 3

private val TIMESTAMP_FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HHmmss.n").withZone(ZoneId.of("UTC"))

// Note: The "(File: ...)" part in the patterns below is actually added by our own getRawResult() function.
// Note: The "(File: ...)" part in the patterns below is actually added by ORT's own getRawResult() function.
private val UNKNOWN_ERROR_REGEX = Regex(
"(ERROR: for scanner: (?<scanner>\\w+):\n)?" +
"ERROR: Unknown error:\n.+\n(?<error>\\w+Error)[:\n](?<message>.*) \\(File: (?<file>.+)\\)",
Expand Down
3 changes: 1 addition & 2 deletions utils/common/src/main/kotlin/DirectoryStash.kt
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ fun stashDirectories(vararg directories: File): Closeable = DirectoryStash(setOf
*/
private class DirectoryStash(directories: Set<File>) : Closeable {
private val stashedDirectories: Map<File, File?> = directories.associateWith { originalDir ->
// We need to check this on each iteration instead of filtering beforehand to properly handle parent / child
// directories.
// Check this on each iteration instead of filtering beforehand to properly handle parent / child directories.
if (originalDir.isDirectory) {
// Create a temporary directory to move the original directory into as a sibling of the original directory
// to ensure it resides on the same file system for being able to perform an atomic move.
Expand Down
4 changes: 2 additions & 2 deletions utils/ort/src/main/kotlin/Utils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ fun filterVersionNames(version: String, names: List<String>, project: String? =
}

return filteredNames.filter {
// startsWith("") returns "true" for any string, so we get an unfiltered list if "project" is "null".
// startsWith("") returns "true" for any string, so this yields an unfiltered list if "project" is "null".
it.startsWith(project.orEmpty())
}.let {
// Fall back to the original list if filtering by project results in an empty list.
Expand Down Expand Up @@ -176,7 +176,7 @@ fun normalizeVcsUrl(vcsUrl: String): String {
}
}

// If we have no protocol by now and the host is Git-specific, assume https.
// If there is no protocol by now and the host is Git-specific, assume https.
if (url.startsWith("github.com") || url.startsWith("gitlab.com")) {
url = "https://$url"
}
Expand Down
2 changes: 1 addition & 1 deletion utils/ort/src/test/kotlin/OrtProxySelectorTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class OrtProxySelectorTest : WordSpec({
})

private fun createProxySelector(protocol: String): OrtProxySelector {
// Using a non-null assertion is fine here as we know the URL to be parsable.
// Using a non-null assertion is fine here as the URL is known to be parsable.
val proxy = determineProxyFromUrl("http://fake-proxy:8080")!!

return OrtProxySelector().removeAllProxies().addProxy("test", protocol, proxy)
Expand Down
Loading