search

sillsdev / libpalaso

Palaso Library: A set of .Net libraries useful for developers of Language Software.
MIT License
44 stars 50 forks source link

Renamed SIL.Media to SIL.Windows.Forms.Media #1325

Open josephmyers opened 4 months ago

josephmyers commented 4 months ago

This change is Reviewable

tombogle commented 2 months ago

I wonder if it might be best to split SIL.Media into a utility DLL and a Forms DLL, as we have with the other things. There are only a small handful of WinForms-specific things here.

github-actions[bot] commented 2 months ago

LibPalaso Tests

   17 files  ±0     17 suites  ±0   8m 33s :stopwatch: +22s 4 883 tests ±0  4 652 :white_check_mark: ±0  231 :zzz: ±0  0 :x: ±0  4 902 runs  ±0  4 658 :white_check_mark: ±0  244 :zzz: ±0  0 :x: ±0 

Results for commit 0b7a9aff. ± Comparison against base commit db9df03c.

josephmyers commented 2 months ago

If it's a small handful, would there be any value in absorbing it into an existing library? Is the benefit of a separate dll enough to justify its cost?

tombogle commented 2 months ago

I don't see any logical fit anywhere else. The only possible place would be Core, but everything uses thats, so I think it would be less than ideal to put it there. Eventually, HearThis will probably become a non-WinForms app (or get a non-WinForms cousin), so breaking the functionality up really does make sense. I don't know enough about the architecture of Bloom, but it would be another possible candidate for using stuff in SIL.Media if it were not WinFoms dependent.

josephmyers commented 2 months ago

I'm also fine leaving it as is. The only thing this PR does is make the name less misleading.