PyCQA / eradicate

Removes commented-out code from Python files
https://pypi.python.org/pypi/eradicate
206 stars 25 forks source link

Add windows installation compatibility #17

Closed asimon-1 closed 1 year ago

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.4%) to 99.618% when pulling 7bc7fb77a2e5b5b113775f5dd828a59e2b9ebdc3 on scuriosity:master into 522ed7ce2da82d33b3e2331bf50d4671c5a5af9a on myint:master.

myint commented 4 years ago

Thanks! I can merge this in once the functionality in the existing eradicate script is somehow merged into this.

CAM-Gerlach commented 3 years ago

@asimon-1 @myint At least running under pre-commit, the current version (2.0.0) of Eradicate is still broken and unusable for me on Windows (failing on run with error Executable `C:UsersC.` not found; there's a space in my path which is why it likely isn't showing the whole thing, but its broken regardless since the \s are interpreted as escapes). Furthermore, it appears it relies on ancient and deprecated distutils functionality that will be removed and thus stop working for everyone in the Python version after next, which this PR resolves. Could you outline the blocker(s) to getting this critical PR merged? Thanks!

sobolevn commented 1 year ago

Hi everyone! ๐Ÿ‘‹ Especially @CAM-Gerlach :)

Do you still need this PR? In case you do, please rebase / recreate it. I will be more than happy to merge it and add Windows CI.

CAM-Gerlach commented 1 year ago

Hi @sobolevn ; cool to see you here! Haven't been really following development of this so not 100% sure the specific blocker is here, but given there are no apparent conflicts (assuming GitHub is reporting that correctly) it would seem that the changes are still needed, and if it just needs a rebase any maintainer should be able to trigger it either manually or with this option in the main repo settings page:

image

If further non-trivial changes are needed perhaps I could make them as suggestions if possible, though at that point might be worth just remaking the PR to fix/clean up some things and add the CIs. Not 100% sure I'll have the bandwidth but I could potentially try to give it a shot at some point if someone else doesn't get to it first...

CAM-Gerlach commented 1 year ago

Also, I couldn't resist the opportunity considering our typical respective roles in the past, haha...

image

sobolevn commented 1 year ago

@CAM-Gerlach this is very funny ๐Ÿ˜†

I cannot change the project settings. But I can re-open this PR. I think that GitHub does the rebase automatically in this case.

There are some conflicts for sure, for example: https://github.com/myint/eradicate/blob/master/setup.py#L8

asimon-1 commented 1 year ago

Hi all - thanks for reopening this! I haven't been involved with the guts of python distribution in some time, so it'd be better if someone else handled the rebase conflicts.

I remember this being a pretty small change so hopefully it will be easy to address.

CAM-Gerlach commented 1 year ago

I cannot change the project settings.

If you're the lead maintainer, that's a longer-term issueโ€”you might want to consider asking @myint to move the project to a GitHub org with shared ownership to reduce bus factor.

But I can re-open this PR. I think that GitHub does the rebase automatically in this case.

Hmm, I don't think so but it will re-run the checks.

There are some conflicts for sure, for example: master/setup.py#L8

Hmm, in that case GitHub is likely screwing up and saying it doesn't because the PR is too old...yeah, best to create a new one. I'll add it to my queue and take a look if I get the time, given @asimon-1 is okay with it, unless you get to it first.

asimon-1 commented 1 year ago

All yours

sobolevn commented 1 year ago

@myint we can move this to https://github.com/wemake-services/

It has several flake8 plugins already, I will give you admin access on the repo. License / credits will stay the same, of course. Old address will just redirect to a new one.

myint commented 1 year ago

@sobolevn Sure, that works for me.

sobolevn commented 1 year ago

@myint can you please first send a transfer request for my personal account? This way I won't need to add you to the org. Because "create repo" permission is required for an org transfer.

I will transfer it to the org.

Thanks a lot! ๐Ÿ‘

myint commented 1 year ago

Screen Shot 2023-03-02 at 07 24 29

sobolevn commented 1 year ago

@myint I've removed my fork, please try again.

myint commented 1 year ago

Done:

Repository transfer to sobolevn requested

sobolevn commented 1 year ago

๐ŸŽ‰

New home is here :)

sobolevn commented 1 year ago

@myint please check your admin permissions. Does it work? Anything else we have to do? :)

CAM-Gerlach commented 1 year ago

Great, thanks @myint and @sobolevn ! With the additional motivation of your effort here, I'll try to take a look at this next week.

And @sobolevn I had no idea all this time that you were the guy behind the famous WeMake! I've actually used your linter config and (at least part of) your style guide on some of my repos.

sobolevn commented 1 year ago

@CAM-Gerlach awesome to hear that! I know that it might be way too strict sometimes, but I see great results of it tutoring my students.

I've enabled the "rebase" button for this repo. Testing it out.

myint commented 1 year ago

@sobolevn Things look good to me. Thanks!

sobolevn commented 1 year ago

It does not feel right to me (see https://github.com/wemake-services/eradicate/pull/45 for more context) to store this package under my org, I want to pay more respect to the original author. Since wemake-services once was a for-profit company, it complicates things even more for me.

Maybe we can try moving it to https://github.com/PyCQA ? I think it is significant enough to be accepted and since @myint is a member of PyCQA, it should not be a problem. I will send an email today, if you support my idea, please reply to https://mail.python.org/mailman3/lists/code-quality.python.org/

Cheers!

sobolevn commented 1 year ago

Sorry for the extra noise!

sobolevn commented 1 year ago

I got a "yes" from PyCQA: https://mail.python.org/archives/list/code-quality@python.org/thread/33SZLUQZJFUPK3OAP7DJCIHTTDEOSSNG/

@myint what do you think? :) Do you approve it?

sobolevn commented 1 year ago

I will close this issue, because 2.3 with Windows support was just released, but I am still waiting for @myint's response about the project transfer :)

myint commented 1 year ago

@sobolevn Moving it to PyCQA sounds great to me!

sobolevn commented 1 year ago

Done, I hope for the final time :)