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

Fix a JET warning #44

Merged
merged 3 commits into from
Nov 1, 2023
Merged

Fix a JET warning #44

merged 3 commits into from
Nov 1, 2023

Conversation

fingolfin
Copy link
Member

In the 'else' path, JET cannot prove that SCRATCH_DIR_OVERRIDE[] is
(still) 'nothing', and thus reports a possible error. This then is also
shown when analyzing packages that use Scratch. This can be resolved by
assigning it first to a temporary variable.

In the 'else' path, JET cannot prove that SCRATCH_DIR_OVERRIDE[] is
(still) 'nothing', and thus reports a possible error. This then is also
shown when analyzing packages that use Scratch. This can be resolved by
assigning it first to a temporary variable.
@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

Merging #44 (90ebed6) into master (7df2dd3) will increase coverage by 0.11%.
Report is 1 commits behind head on master.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
+ Coverage   87.61%   87.73%   +0.11%     
==========================================
  Files           1        1              
  Lines         105      106       +1     
==========================================
+ Hits           92       93       +1     
  Misses         13       13              
Files Coverage Δ
src/Scratch.jl 87.73% <66.66%> (+0.11%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@fingolfin
Copy link
Member Author

Hmm, seems Scratch is broken on Julia nightly?

@fingolfin
Copy link
Member Author

@staticfloat @fredrikekre @giordano any concerns about this PR? Otherwise I'd like to merge it and make a new release, to get my package "JET clean" ;-)

@giordano
Copy link
Member

Looks good to me.

@staticfloat
Copy link
Member

Looks fine to me; shall we add a JET pass to the CI in a follow-on PR so that we don't accidentally regress it?

@fingolfin
Copy link
Member Author

@staticfloat I've added JET to the test suite, let me know if that's what you had in mind.

test/runtests.jl Outdated Show resolved Hide resolved
@fingolfin fingolfin merged commit 465b5fa into master Nov 1, 2023
8 of 12 checks passed
@fingolfin fingolfin deleted the mh/JET branch November 1, 2023 20:06
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.

4 participants