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 warnings #213

Merged
merged 4 commits into from
May 26, 2023
Merged

Fix warnings #213

merged 4 commits into from
May 26, 2023

Conversation

gilch
Copy link
Owner

@gilch gilch commented May 26, 2023

Fixes #212.

Also configures warnings to fail the workflow, which would have caught this issue, which I confirmed in https://github.com/gilch/hissp/actions/runs/5086943652/jobs/9141865517.

Turns out deprecation warnings don't even print by default. This may turn out to be too strict, but we'll cross that bridge when we come to it.

Flags not at the start of the expression '\\n$|(?m)^ *;+ ?'
@gilch
Copy link
Owner Author

gilch commented May 26, 2023

Well, we got there pretty quickly. The warning seems to be in the test runner itself. Maybe I can tweak the config a bit, or ignore warnings just for that package. Might take a few experiments.

@codecov
Copy link

codecov bot commented May 26, 2023

Codecov Report

Merging #213 (0ee0f12) into master (6be47da) will decrease coverage by 0.29%.
The diff coverage is 100.00%.

❗ Current head 0ee0f12 differs from pull request most recent head f55da8d. Consider uploading reports for the commit f55da8d to get more accurate results

@@             Coverage Diff             @@
##            master     #213      +/-   ##
===========================================
- Coverage   100.00%   99.71%   -0.29%     
===========================================
  Files            6        6              
  Lines          706      704       -2     
  Branches       111      110       -1     
===========================================
- Hits           706      702       -4     
- Misses           0        1       +1     
- Partials         0        1       +1     
Impacted Files Coverage Δ
src/hissp/repl.py 96.55% <ø> (-3.45%) ⬇️
src/hissp/reader.py 100.00% <100.00%> (ø)

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

@gilch
Copy link
Owner Author

gilch commented May 26, 2023

Wow, first try. I'm going to swap the order so I don't merge in failing commits.

@gilch
Copy link
Owner Author

gilch commented May 26, 2023

Hmm. I maybe lost coverage on a one-line branch in the repl module. That may be dead code after removing that entry point. __main__ does that on its own. If you're calling it manually rather than via __main__, you should probably call hissp.repl.interact() instead.

I'll try removing the default for the __main__ argument to eliminate the branch.

gilch added 3 commits May 25, 2023 22:47
 'hissp.repl' found in sys.modules after import of package
 'hissp', but prior to execution of 'hissp.repl'; this may result in
 'unpredictable behaviour

Remove direct repl entry point. It is no longer required.

Use hissp's __main__.py instead.
@gilch gilch merged commit b733a43 into master May 26, 2023
@gilch gilch deleted the fix-warnings branch May 26, 2023 04:52
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.

Build broke on Python 3.11
1 participant