chore: switch to dirs.dev library #1482
Conversation
- Update version catalog to use official dev.dirs:directories:26 - Add directories-jvm imports to RuntimeWorkdirManager - Replace custom directory logic with dirs.dev providers - Simplify userConfigDirectory and cachesDirectory implementations - Comment out platform-specific helper functions Progress towards #1286
sgammon
added
enhancement
claspell
changed the title
- Build successful with 'elide build' - Functionality verified with 'elide info' - All directory paths resolving correctly - Ready for review Fixes #1286
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1482 +/- ##
==========================================
- Coverage 41.99% 40.91% -1.08%
==========================================
Files 680 692 +12
Lines 31366 32335 +969
Branches 4386 4519 +133
==========================================
+ Hits 13172 13230 +58
- Misses 16627 17532 +905
- Partials 1567 1573 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 28 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
claspell
changed the title
|
@sgammon Ready for review Should I clean up the commented-out legacy code, or do you want to keep it for reference during review? |
| dirs = "bf76af41b9" | ||
| dirs = "26" |
There was a problem hiding this comment.
this override was in place because we needed to fork the lib to build with updated deps (i think?). anyway, it's probably worth keeping, or figuring out why we forked and making a decision based on that (if we forked to add features, are those features worth keeping? for fixes, are the fixes still needed?)
| // private const val nixTempPath = "/tmp/elide-runtime" | ||
| // private const val runtimeDirPrefix = "elide-runtime-" | ||
| // private const val elideHomeDirectory = ".elide" | ||
| //private const val elideConfigDirectory = "elide" | ||
| //private const val configDirectory = ".config" | ||
|
|
||
| // private val linuxCachesPath = "~/elide/caches/v${Elide.version()}" | ||
| // private val darwinCachesPath = "/Library/caches/elide/v${Elide.version()}" |
There was a problem hiding this comment.
const val must be listed literally, because it's "constant." on the other hand, const can be dropped and you can just use @JvmStatic val, which is a "static value," that is computed at program startup time, instead of build time.
| // private fun currentSharedTempPrefix(): String { | ||
| // return StringBuilder().apply { | ||
| // append(runtimeDirPrefix) | ||
| // append(Elide.version()) | ||
| // }.toString() | ||
| // } |
There was a problem hiding this comment.
this will get easier to review, and easier to work with, if you remove code instead of commenting. i get the impulse
| private fun obtainWorkdir(): Path { | ||
| if (!this::rootDirectory.isInitialized) { | ||
| rootDirectory = Path.of(projectDirectories.cacheDir) | ||
| if (!rootDirectory.exists()) { | ||
| Files.createDirectories(rootDirectory) | ||
| } | ||
| } | ||
| return rootDirectory | ||
| } |
There was a problem hiding this comment.
your code here improves on what was here, but the approach could use some tuning. i wonder if we can avoid checking the existence / creating the path on first access? maybe we can create a wrapper type for this? ill connect with you on discord about it
Summary
Replace custom
RuntimeWorkdirManagerdirectory logic with the standarddirs.devlibrary for better cross-platform directory handling.Changes
dirs.devfactory methodsobtainWorkdir()andworkSubdir()methodsImpact
Fixes #1286
Testing