-
Notifications
You must be signed in to change notification settings - Fork 60
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
Remove deprecated functions and parameters #245
Conversation
Refactor tests to use _get_install_ids
External URL's in removed notes seem to be broken tree_libs tests seem redundant with adjacent tests _allow_all is only used in delocating.py now
Refactor get_archs, now accepts PathLike and raises FileNotFoundError
Coverage xml is automatic and does not need its own step
Commands must be an explicit sequence, similar to subprocess
Command main functions are covered but these branches can not be covered by the script testing tool.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #245 +/- ##
==========================================
+ Coverage 96.92% 97.43% +0.50%
==========================================
Files 16 16
Lines 1366 1246 -120
==========================================
- Hits 1324 1214 -110
+ Misses 42 32 -10 ☔ View full report in Codecov by Sentry. |
3777b9b
to
95b56ba
Compare
How long have these functions been deprecated for (in terms of time, and releases)? |
Some are very recent, some are older. Some of them are documented with Many of these functions have a broken implementation which is either not aware of recursive libraries or is not aware of rpaths or Removed/changed functions:
Not removed:
Times of releases with times of deprecation:
|
In general, I am sure that a) not many people are using the internal functions, and b) those that are will tend to be release managers, so able to adapt quickly. So fast deprecation / removal cycles are OK, if that is what you prefer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - comment on change in Exception.
What do you mean by this? The change to get_archs is already mentioned in the changelog. |
I was referring to my comment on the change. It was only to check that it
would have minimal impact.
…On Thu, 30 Jan 2025 at 23:43, Kyle Benesch ***@***.***> wrote:
comment on change in Exception.
What do you mean by this? This change is already mentioned in the
changelog.
—
Reply to this email directly, view it on GitHub
<#245 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAQQHADHNYLT2RIIVD32N32NK2JHAVCNFSM6AAAAABWF7KYNCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMRVHE2DSNRWGU>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Removes a significant amount of code which was publicly marked as deprecated in the latest release. This should make the rest of the code easier to maintain. All deprecated features have been removed except for
get_rpaths
since_get_rpaths
needs more work to replace it in tests.Refactors
get_archs
with changes noted in the changelog.Improves the output of pytest in CI a little bit by showing which lines are uncovered. I've marked the
if __name__ == "__main__":
branches in commands as uncoverable.Each removal is its own commit in case anything needs to be reverted, edited, or split into separate PR's.
Pull Request Checklist