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

Add windows installation compatibility #17

Closed
wants to merge 3 commits into from

Conversation

asimon-1
Copy link

No description provided.

@coveralls
Copy link

Coverage Status

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

@myint
Copy link
Member

myint commented Dec 23, 2019

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

@CAM-Gerlach
Copy link

CAM-Gerlach commented Aug 13, 2021

@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
Copy link
Member

sobolevn commented Mar 1, 2023

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
Copy link

CAM-Gerlach commented Mar 1, 2023

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
Copy link

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

image

@sobolevn
Copy link
Member

sobolevn commented Mar 1, 2023

@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

@sobolevn sobolevn closed this Mar 1, 2023
@sobolevn sobolevn reopened this Mar 1, 2023
@asimon-1
Copy link
Author

asimon-1 commented Mar 1, 2023

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
Copy link

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
Copy link
Author

asimon-1 commented Mar 2, 2023

All yours

@sobolevn
Copy link
Member

sobolevn commented Mar 2, 2023

@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
Copy link
Member

myint commented Mar 2, 2023

@sobolevn Sure, that works for me.

@sobolevn
Copy link
Member

sobolevn commented Mar 2, 2023

@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
Copy link
Member

myint commented Mar 2, 2023

Screen Shot 2023-03-02 at 07 24 29

@sobolevn
Copy link
Member

sobolevn commented Mar 2, 2023

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

@myint
Copy link
Member

myint commented Mar 3, 2023

Done:

Repository transfer to sobolevn requested

@sobolevn
Copy link
Member

sobolevn commented Mar 3, 2023

🎉

New home is here :)

@sobolevn
Copy link
Member

sobolevn commented Mar 3, 2023

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

@CAM-Gerlach
Copy link

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
Copy link
Member

sobolevn commented Mar 3, 2023

@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
Copy link
Member

myint commented Mar 3, 2023

@sobolevn Things look good to me. Thanks!

@sobolevn
Copy link
Member

sobolevn commented Jun 8, 2023

It does not feel right to me (see #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
Copy link
Member

sobolevn commented Jun 8, 2023

Sorry for the extra noise!

@sobolevn
Copy link
Member

sobolevn commented Jun 8, 2023

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
Copy link
Member

sobolevn commented Jun 9, 2023

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 :)

@sobolevn sobolevn closed this Jun 9, 2023
@myint
Copy link
Member

myint commented Jun 10, 2023

@sobolevn Moving it to PyCQA sounds great to me!

@sobolevn
Copy link
Member

Done, I hope for the final time :)

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.

5 participants