-
Notifications
You must be signed in to change notification settings - Fork 10
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 obsolete files #340
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev_master #340 +/- ##
===========================================
Coverage 77.04% 77.04%
===========================================
Files 57 57
Lines 7693 7693
===========================================
Hits 5927 5927
Misses 1766 1766 ☔ View full report in Codecov by Sentry. |
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.
Hmm I already wrote a comment here, where I included the commits that introduced these OLD files. As in, that might make it easier to figure out whether there is something in them worth keeping. But can't find that comment.
Anyway, it was still to complicated to figure out whether there was still something worthwhile in there.. You'd need to ask Kieran I think.
Personally I'm in favor of removing this old code.
In AstarVienna/speXtra#13 you grumbled about coverage going down by disabling coverage on test files. Then I realized that coverage should have gone up here, because we are removing uncovered files, but it didn't. Apparently we don't count untested files in our coverage. Perhaps we should count all not-explicitly excluded Python files? Or maybe have a test that simply tries to import all Python files? I looked around a bit, and so far found one other: https://github.com/AstarVienna/ScopeSim/blob/dev_master/scopesim/detector/nghxrg.py . The only place it was used was, you might have guessed:
And indeed, it is broken:
Maybe I'll give it a quick shot to try to write a test to import all Python modules. Not sure what to do with |
Specifically about this |
Created a tag on dev_master just before this branch-off, so should we ever need anything from those old files, we can just checkout that.
Also deleted MANIFEST.in, which is now obsolete (I think) and new_features.rst, which was hopelessly outdated anyway (use CHANGLOG.md in the future for this...).
Any reservations against this move?