Simplify workload_enabled

[prbzrg]

No description provided.

[timholy]

Functionally, the only difference here is the removal of the try/catch. Is there justification for that removal?

[prbzrg]

Is try/catch needed? If there is a reason for it, we should print a warning to avoid silent failure.

[timholy]

git blame is your friend here, it pointed me to #11

[prbzrg]

Thanks for the hint, I added a warning.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 75.58%. Comparing base (a994463) to head (9de5143).

Files	Patch %	Lines
src/workloads.jl	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main      #38       +/-   ##
===========================================
- Coverage   91.66%   75.58%   -16.09%     
===========================================
  Files           3        3               
  Lines          96       86       -10     
===========================================
- Hits           88       65       -23     
- Misses          8       21       +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[timholy]

I guess what might help here is some explanation of why this seems like an important change. Sure, it's a little cleaner and shorter. But it changes functionality (now you get a warning), and changes to PrecompileTools affect the entire ecosystem. If you're changing the experience of more than a million people, it would be great to have a bit more justification than "No description provided." Under what circumstances does the current absence of a warning cause problems? How frequently will the warning be triggered? etc.

[prbzrg]

The reason for this PR was I disabled precompile_workloads and it didn't trigger any precompilation (assuming it changes part of packages), then I suspected and looked into the codes. I decided to simplify it and remove try/catch, to make sure it works as expected.
It's a simple change, and I agree with you that it can have a massive effect in the user base!
If it doesn't worth it, feel free to close this PR.

[timholy]

Thanks for the explanation. If I have a little "general Julia" development time this summer, I'll try checking out this branch and see what my experience is. Until then, let's leave it open. Also, please do report any experiences you have on triggering that warning, if you're using your own branch.

[timholy]

I disabled precompile_workloads and it didn't trigger any precompilation (assuming it changes part of packages)

That's a natural confusion. While it does change the package, the old cache file is still valid. Essentially, the cache file without precompiling the workload is strictly a subset of the cache file when you do precompile the workload. Since there's nothing wrong with having the more expansive cache, it doesn't trigger recompilation.

That's different from, say, recompiling with code-coverage, where the generated code is actually different and so the old cache file isn't valid.

[prbzrg]

If this isn't beneficial, feel free to close it.

[timholy]

It might be useful, so if it's OK with you I'd like to leave it open. It seems like something that might be worth testing more extensively in conjunction with other changes, e.g., #42 (comment). If you're going to make some changes, maybe best to make several all at once and then be ready to fix things.