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

Remove support for Python 3.6 (it's "end of life" December 23, 2021) #2141

Merged
merged 2 commits into from Dec 19, 2021

Conversation

barrywhart
Copy link
Member

@barrywhart barrywhart commented Dec 19, 2021

Brief summary of the change made

Removes support for Python 3.6.

Are there any other side effects of this change that we should be aware of?

Pull Request checklist

  • Please confirm you have completed any of the necessary steps below.

  • Included test cases to demonstrate any code changes, which may be one or more of the following:

    • .yml rule test cases in test/fixtures/rules/std_rule_cases.
    • .sql/.yml parser test cases in test/fixtures/dialects (note YML files can be auto generated with python test/generate_parse_fixture_yml.py or by running tox locally).
    • Full autofix test cases in test/fixtures/linter/autofix.
    • Other.
  • Added appropriate documentation for the change.

  • Created GitHub issues for any relevant followup/future enhancements if appropriate.

@codecov
Copy link

codecov bot commented Dec 19, 2021

Codecov Report

Merging #2141 (0cf593f) into main (f41c5d1) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              main     #2141   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          148       148           
  Lines        10674     10674           
=========================================
  Hits         10674     10674           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f41c5d1...0cf593f. Read the comment docs.

Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see another couple of references:

image

Or we happy to leave those in there and continue to support 3.6 unofficially?

Also FYI, in .github/codecov.yml we have this:

  notify:
    after_n_builds: 11

As of this PR:

  • 4 linux standard runs are eligible 3.7 - 3.10
  • 5 linux DBT runs (dbt018 - dbt100)
  • 1 windows standard run runs
  • 1 windows dbt run

Windows Linux is usually last to finish and think we now need it as a bit of coverage depends on that.

So think this is OK to leave. Or could reduce to 10 to get early feedback of code coverage but then you sometimes get false negatives of missing coverage.

@barrywhart
Copy link
Member Author

Last night, I removed those 2 other 3.6 references, and some tests were failing -- some weird pickling error in the progress bar tests. I'm guessing it was the stuff in runner.py. So I'd prefer to leave that for now if that's okay.

@barrywhart
Copy link
Member Author

I'm good with leaving it at 11. False positives are 😿

@barrywhart barrywhart merged commit 0b1ec94 into sqlfluff:main Dec 19, 2021
@jpy-git jpy-git mentioned this pull request Dec 19, 2021
1 task
@jpy-git
Copy link
Contributor

jpy-git commented Dec 19, 2021

I'm good with leaving it at 11. False positives are 😿

Was wondering why we kept getting coverage errors before all the tests had finished since we added dbt100. Glad this will start working properly again 🎉

@tunetheweb
Copy link
Member

Yeah it’s annoying you have to remember to change this. But annoying. We used to have more headroom when Windows standard wasn’t required so didn’t matter it was the last to complete. I wonder if there’s a better way than just setting the number and telling codecov. Would be nice if we could say at least I’ve standard, dbt and windows test has to run.

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