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.

Sign up for GitHub

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

Jump to bottom

added encoding to s3_copy function in redshift class #827

Merged
merged 8 commits into from
May 23, 2024
Merged

added encoding to s3_copy function in redshift class #827

merged 8 commits into from
May 23, 2024

Conversation

sharinetmc
Copy link
Contributor

No description provided.

willyraedy
willyraedy reviewed Jun 1, 2023
View reviewed changes
Copy link
Contributor

@willyraedy willyraedy left a comment

Choose a reason for hiding this comment

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

Looks like the linter may be flagging a problem which we'll want to pass before we merge this but other than that, looks good.

shaunagm
shaunagm requested changes Jun 23, 2023
View reviewed changes
Copy link
Collaborator

@shaunagm shaunagm left a comment

Choose a reason for hiding this comment

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

Need to be linted by running Black but otherwise should be ready to go (relying on Willy's review in the comments)

@sharinetmc sharinetmc requested a review from shaunagm June 26, 2023 19:03
@shaunagm
Copy link
Collaborator

So the instructions for how to use Black in the docs are wrong, whoops. Can you try black --check parsons/ test/ useful_resources/

@bmos bmos mentioned this pull request Mar 31, 2024
Merged
bmos and others added 2 commits April 4, 2024 16:31
@bmos @SorenSpicknall
Fix failing linting test in #827 (#1020)
2f5e52b 
* fix black error

* Bump Multiple Docs Deps Around Sphinx (#975)

* Bump multiple docs deps

* Downgrade myst-parser

---------

Co-authored-by: Soren Spicknall <soren@spicknall.us>
@shaunagm
Merge branch 'main' into add_encoding_arg_to_copy_s3_func
0fb5ab3
@shaunagm
Copy link
Collaborator

shaunagm commented Apr 4, 2024

@sharinetmc can you double-check that I resolved the merge conflict correctly and that the function does what it should? 0fb5ab3

If you can confirm that, we can merge this

@sharinetmc
Copy link
Contributor Author

@shaunagm, tests are all passing now and everything looks good!

shaunagm
shaunagm approved these changes May 23, 2024
View reviewed changes
Copy link
Collaborator

@shaunagm shaunagm left a comment

Choose a reason for hiding this comment

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

LGTM

@shaunagm shaunagm merged commit 0ec8a34 into main May 23, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shaunagm shaunagm shaunagm approved these changes

@movecoop movecoop Awaiting requested review from movecoop

@willyraedy willyraedy Awaiting requested review from willyraedy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sharinetmc @shaunagm @willyraedy @bmos