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: remove warning on 3.12a7+ #529

Merged
merged 2 commits into from Jul 3, 2023
Merged

Conversation

henryiii
Copy link
Contributor

Removing warning on 3.12 alphas. Closes #526.

This drops support for the early 3.12 alphas, but older alphas aren't usually actively supported anyway. LMK if they need support too.

@agronholm
Copy link
Contributor

Please add a news item too.

@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Patch coverage: 40.00% and project coverage change: +0.17 🎉

Comparison is base (e18afcf) 70.02% compared to head (5d216c5) 70.20%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #529      +/-   ##
==========================================
+ Coverage   70.02%   70.20%   +0.17%     
==========================================
  Files          13       13              
  Lines        1071     1074       +3     
==========================================
+ Hits          750      754       +4     
+ Misses        321      320       -1     
Impacted Files Coverage Δ
src/wheel/bdist_wheel.py 47.26% <40.00%> (+0.89%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@agronholm
Copy link
Contributor

Should we perhaps add a test for the callback? I know we didn't have one, but if you feel inclined to add one, that would be wonderful. It's not a showstopper though.

@henryiii
Copy link
Contributor Author

I'll look into it soon! PyCon might get in the way of doing it promptly.

@henryiii henryiii force-pushed the henryiii/fix/312warn branch 3 times, most recently from 76b08ca to d4e443f Compare June 14, 2023 18:55


def remove_readonly_exc(func, path, exc):
print(str(exc))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prints to stdout. Shouldn't it print to stderr instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it should print anything at all. In all likelihood this was a leftover debugging line.

@henryiii henryiii marked this pull request as ready for review June 14, 2023 19:00
@henryiii
Copy link
Contributor Author

henryiii commented Jun 14, 2023

FYI, there is a way to set patch coverage to 'informational' to get it to report on the diff, but not put a red x if an uncovered line is added or changed. Let me know if you are interested in me adding that. :)

@henryiii
Copy link
Contributor Author

@agronholm, any comments?

@agronholm
Copy link
Contributor

FYI, there is a way to set patch coverage to 'informational' to get it to report on the diff, but not put a red x if an uncovered line is added or changed. Let me know if you are interested in me adding that. :)

Yes, please.

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Copy link
Contributor

@agronholm agronholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nits, otherwise OK.

docs/news.rst Outdated Show resolved Hide resolved


def remove_readonly_exc(func, path, exc):
print(str(exc))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it should print anything at all. In all likelihood this was a leftover debugging line.

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
@agronholm
Copy link
Contributor

agronholm commented Jul 3, 2023

There's a new checkbox checked by default: [ ] Wait for successful checks. If that's checked, the Confirm squash and merge button is just disabled after pressing and nothing happens despite all checks already passing. What am I missing here?

@henryiii
Copy link
Contributor Author

henryiii commented Jul 3, 2023

I've never seen that. Are you using Refined-github? A search shows refined-github/refined-github#1771. But it doesn't look like that, it's integrated in to the button.

GitHub's built-in feature is based on branch protections (and ideally, a "pass" job).

@agronholm agronholm merged commit f2af601 into pypa:main Jul 3, 2023
18 checks passed
@agronholm
Copy link
Contributor

Yeah, must be that. I completely forgot that I still had that extension.

@henryiii henryiii deleted the henryiii/fix/312warn branch July 3, 2023 19:11
@henryiii
Copy link
Contributor Author

Any plans for a patch release? It would be nice to have this warning fix & the build "number" fix. :)

@agronholm
Copy link
Contributor

Done.

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.

onerror now producing warnings in Python 3.12 latest alpha
2 participants