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

Drop code that explicitly supported python 2. #269

Merged
merged 6 commits into from
May 4, 2023

Conversation

sodul
Copy link
Collaborator

@sodul sodul commented Apr 25, 2023

Tested with green green with python 3.6 (some of the test code uses f-strings so we cannot test with python 3.5).

Tested with `green green` with python 3.6 (some of the test code uses f-strings so we cannot test with python 3.5).
@CleanCut
Copy link
Owner

It's the end of an era. 😢

Time to move on! This is looking pretty good. ❤️

Some minor things to address:

  • Fix the linting errors from black
  • Update the changelog
  • Update the readme

If you will enable the Allow edits from maintainers setting on this PR, I can fixup minor issues like these for you instead.

@sodul
Copy link
Collaborator Author

sodul commented Apr 26, 2023

Sorry, the Allow edits from maintainers option does not exist for me here.

image

@sodul
Copy link
Collaborator Author

sodul commented Apr 26, 2023

I've run black 23.3.0 (latest), added Python 3.11 to the CI checks and updated the .md files.

Note that I mentioned the wrong version of black in my commit ... it was the version of Pip, which is also 23.x.

@CleanCut
Copy link
Owner

Sorry, the Allow edits from maintainers option does not exist for me here.

Huh. Weird. Do you see it in a place more like this?

image

@CleanCut
Copy link
Owner

I've run black 23.3.0 (latest),

That's really odd, because you're commit has lots of whitespace changes that are showing up as new linting errors, and when I run black locally, it reverts your changes. Do you, perhaps, have some different black configuration in your local environment or something?

CHANGELOG.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
sodul and others added 3 commits April 26, 2023 16:33
Co-authored-by: Nathan Stocks <cleancut@github.com>
Co-authored-by: Nathan Stocks <cleancut@github.com>
```
docker run -it --entrypoint=/bin/bash -v $(pwd):/green python:3.11
root@bdf5d7007526:/# cd green/
root@bdf5d7007526:/green# pip install black
root@bdf5d7007526:/green# black green
root@bdf5d7007526:/green# black example
```
@sodul
Copy link
Collaborator Author

sodul commented Apr 26, 2023

Sorry, I really cannot find the checkbox to allow edits anywhere. The fork is tied to our work Org and might not give the option somehow.

I've added your suggestions to the PR.

As for black, I re-ran it from a vanilla image to remove the possibility of an other config on my machine. I hope this time it will be better.

We usually run gray on our codebase and not black as it is more flexible. We do have it call black for the general whitespace and line splitting handling, but we prefer single quotes for example.

@sodul sodul requested a review from CleanCut April 27, 2023 00:01
@sodul
Copy link
Collaborator Author

sodul commented May 2, 2023

@CleanCut is there anything else you want me to change here?

@coveralls
Copy link

Coverage Status

Coverage: 99.918% (-0.08%) from 100.0% when pulling 5835e00 on clumio-oss:drop_python_v2 into 9483a2e on CleanCut:main.

@CleanCut
Copy link
Owner

CleanCut commented May 4, 2023

Sorry for the delay, I was away for several days. 😄

This looks great!

@CleanCut CleanCut merged commit 213854c into CleanCut:main May 4, 2023
@sodul sodul deleted the drop_python_v2 branch May 9, 2023 00:55
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.

None yet

3 participants