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

Release 0.9.0 #2097

Merged
merged 18 commits into from Dec 13, 2021
Merged

Release 0.9.0 #2097

merged 18 commits into from Dec 13, 2021

Conversation

jpy-git
Copy link
Contributor

@jpy-git jpy-git commented Dec 11, 2021

Get ready for release 0.9.0!

In terms of non-draft PRs I believe it is only #2096 that I need to review for @derickl.

2 quick outstanding things that would be good to sort before the release are:

I believe I've correctly updated the CHANGELOG.md as described in CONTRIBUTING.md, but given it's my first time running the release lmk if I've missed anything or if we want anything changed!

@codecov
Copy link

codecov bot commented Dec 11, 2021

Codecov Report

Merging #2097 (7abc58c) into main (c0ac68d) will not change coverage.
The diff coverage is n/a.

❗ Current head 7abc58c differs from pull request most recent head 1e6d4a9. Consider uploading reports for the commit 1e6d4a9 to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##              main     #2097   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          148       148           
  Lines        10640     10640           
=========================================
  Hits         10640     10640           
Impacted Files Coverage Δ
src/sqlfluff/dialects/dialect_tsql.py 100.00% <0.00%> (ø)
src/sqlfluff/dialects/dialect_tsql_keywords.py 100.00% <0.00%> (ø)

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 c0ac68d...1e6d4a9. Read the comment docs.

@jpy-git jpy-git requested review from tunetheweb and barrywhart and removed request for tunetheweb December 11, 2021 20:57
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.

Looks good!

Some small comments and suggestions:

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@barrywhart barrywhart left a comment

Choose a reason for hiding this comment

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

A few minor suggestions around wording. Looks good overall.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
jpy-git and others added 8 commits December 11, 2021 22:52
Co-authored-by: Barry Pollard <barry_pollard@hotmail.com>
Co-authored-by: Barry Pollard <barry_pollard@hotmail.com>
Co-authored-by: Barry Hart <barrywhart@yahoo.com>
Co-authored-by: Barry Hart <barrywhart@yahoo.com>
Co-authored-by: Barry Hart <barrywhart@yahoo.com>
Co-authored-by: Barry Hart <barrywhart@yahoo.com>
@alanmcruickshank
Copy link
Member

@jpy-git - README and short description added ✅ . I haven't managed to find any settings or documentation on how to add an icon to the repository. I've got the sqlfluff icon on the user which I think is all I can do without paying for an account or asking docker really nicely.

If anyone has any other ideas on how to get an icon on it - then I'm all ears!

@jpy-git
Copy link
Contributor Author

jpy-git commented Dec 12, 2021

@alanmcruickshank awesome thanks, the readme looks great.

It looks like to get an icon on the image itself we'd need to apply to make SQLFluff a "Docker Official Image": https://docs.docker.com/docker-hub/official_images/

Reading through those docs it seems like the process is fairly involved so maybe that's something we can leave for when we come out of beta 😊

@tunetheweb
Copy link
Member

I notice the Dockerfile doesn’t include the dbt templater plug-in. Should it? Or is it fair to presume dbt users will all be running it locally as they’ll all be Python users anyway?

@jpy-git
Copy link
Contributor Author

jpy-git commented Dec 12, 2021

@tunetheweb we'll need to build a second docker image for the dbt templater but that's extension work

@tunetheweb
Copy link
Member

Surely both need to exist in the same docker image to be usable? You can't use the templater without SQLFluff.

So thew question is do you need two docker images:

  • A SQLFluff only one
  • A SQLFluff and dbt templater one

Or should we just have one? In which case does it need the dbt templater?

To me the only reason to have two would be to keep the image lighter, but don't think the dbt templater adds that much to the image size (though it does need bits of dbt top so maybe it would).

Anyway agree for now let's go with just the SQLFluff one. As I said above, it may well be a bit overkill having dbt within docker if most dbt users are python users so won't use the Docker image anyway.

@jpy-git
Copy link
Contributor Author

jpy-git commented Dec 12, 2021

You would just have the templater image inherit from the non-templater image

I.e.

FROM sqlfluff/sqlfluff:latest

<dbt templater extras here>

@alanmcruickshank
Copy link
Member

One thing that could make this relatively easy would be to maintain two seperate tags within the same repo rather than two separate docker repositories (a little like how the python image maintains different versions and different debian versions as different tags). I agree that this doesn't need to stop us releasing and it could be follow-on work. It likely would involve some additional machinery anyway to get credentials into the container anyway.

@barrywhart
Copy link
Member

I'm not familiar with using Docker images this way. How will users provide their .sqlfluff, dbt_project.yaml, and other files? Will they use the image directly or inherit from our image?

@jpy-git
Copy link
Contributor Author

jpy-git commented Dec 12, 2021

Let me make an issue to capture these initial thoughts and we can move the discussion there

@tunetheweb
Copy link
Member

I'm not familiar with using Docker images this way. How will users provide their .sqlfluff, dbt_project.yaml, and other files? Will they use the image directly or inherit from our image?

Basically you normally mount your local drive in the image. Something like this:

docker container run -it --rm -v ~/myproject:/myproject --entrypoint="sqlfluff lint /myproject"

Docker runs the command and outputs the result.

So this would be useful for T-SQL developers for example, that normally might not use python and don't want to manage a python environment and all that entails to keep it working and up to date. If they have Docker installed they can just use our Docker image to save them having to run it through a local python install.

@jpy-git jpy-git marked this pull request as ready for review December 13, 2021 20:02
@jpy-git jpy-git merged commit 9504fc2 into sqlfluff:main Dec 13, 2021
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

4 participants