Skip to content

Various common utility refactorings #10181

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

sschuberth
Copy link
Member

Please have a look at the individual commit messages for the details.

@sschuberth sschuberth requested a review from a team as a code owner April 16, 2025 13:08
@sschuberth sschuberth enabled auto-merge (rebase) April 16, 2025 13:09
@sschuberth sschuberth force-pushed the realfile-vs-canonicalfile branch from 613844a to 39d768c Compare April 16, 2025 13:30
@sschuberth

This comment was marked as resolved.

@sschuberth sschuberth marked this pull request as draft April 16, 2025 13:32
auto-merge was automatically disabled April 16, 2025 13:32

Pull request was converted to draft

@sschuberth sschuberth force-pushed the realfile-vs-canonicalfile branch 2 times, most recently from 3a16290 to 29df942 Compare April 17, 2025 11:05
@sschuberth sschuberth marked this pull request as ready for review April 17, 2025 11:05
@sschuberth sschuberth requested review from fviernau and a team April 17, 2025 11:05
@sschuberth sschuberth enabled auto-merge (rebase) April 17, 2025 11:06
@fviernau fviernau disabled auto-merge April 17, 2025 12:28
@sschuberth sschuberth force-pushed the realfile-vs-canonicalfile branch from 29df942 to 381047e Compare April 17, 2025 13:31
@sschuberth sschuberth enabled auto-merge (rebase) April 17, 2025 13:33
@sschuberth sschuberth force-pushed the realfile-vs-canonicalfile branch 2 times, most recently from 7834193 to eb687ee Compare April 17, 2025 13:36
fviernau
fviernau previously approved these changes Apr 17, 2025
Make it `private` now as well and strip docs for this simple function.

Signed-off-by: Sebastian Schuberth <[email protected]>
Testing with `lowercase()` and `uppercase()` is enough.

Signed-off-by: Sebastian Schuberth <[email protected]>
Create various smaller files with more dedicated names for a better
overview. Apply some trivial alignments as part of that, like renaming
of variables or generalizing `Collection` to `Iterable`.

Signed-off-by: Sebastian Schuberth <[email protected]>
This allows for slightly more convenient use.

Signed-off-by: Sebastian Schuberth <[email protected]>
A `realFile` already is absolute.

Signed-off-by: Sebastian Schuberth <[email protected]>
@sschuberth sschuberth force-pushed the realfile-vs-canonicalfile branch from eb687ee to 639b8c5 Compare April 17, 2025 15:27
This makes `common-utils` Jackson-free, which is a step towards being
able to make it a multi-platform project.

Signed-off-by: Sebastian Schuberth <[email protected]>
@sschuberth
Copy link
Member Author

I had to drop the

chore: Prefer realFile over canonicalFile

commit, because actually realFile and canonicalFile serve different purposes: The former resolves symbolic links, the latter resolves superfluous "." in the file path, which could still be a symbolic link.

@sschuberth sschuberth requested review from fviernau and a team April 17, 2025 15:37
@@ -746,7 +746,8 @@ private fun joinNonBlank(vararg strings: String, separator: String = " - ") =
strings.filter { it.isNotBlank() }.joinToString(separator)

private fun IssueTable.title(): String =
"${type.name.titlecase()} Issue Summary ($errorCount errors, $warningCount warnings, $hintCount hints to resolve)"
type.name.lowercase().uppercaseFirstChar() +
Copy link
Member

Choose a reason for hiding this comment

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

FYI, the ORT workbench is using the removed function in several places.

* License-Filename: LICENSE
*/

package org.ossreviewtoolkit.utils.common
Copy link
Member

Choose a reason for hiding this comment

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

Why not keep these in a Utils.kt? We use that name in several modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants