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

[DOM-38006] - Implement black/flake8 code formatter #132

Merged
merged 22 commits into from
Jun 15, 2022
Merged

[DOM-38006] - Implement black/flake8 code formatter #132

merged 22 commits into from
Jun 15, 2022

Conversation

ddl-olsonJD
Copy link
Collaborator

@ddl-olsonJD ddl-olsonJD commented May 26, 2022 •
edited
Loading

Link to JIRA

https://dominodatalab.atlassian.net/browse/DOM-38006

What issue does this pull request solve?

code not following particular formatting guidelines similar to other domino projects

What is the solution?

  • Added flake8 and black implementation to help standardize the code base format.
  • implemented black formatting as base format to continue from
  • implemented pre-commit to run flake8/black against code base based on rules
  • ensure test still runs successfully

Testing

  • Unit test(s)

Pull Request Reminders

References (optional)

Current Test output

image

@ddl-olsonJD ddl-olsonJD marked this pull request as ready for review June 6, 2022 13:56
@ddl-olsonJD ddl-olsonJD requested a review from a team June 6, 2022 13:56
ddl-alexpanin
ddl-alexpanin reviewed Jun 6, 2022
View reviewed changes
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
domino/domino.py Outdated Show resolved Hide resolved
tests/scripts/test_spark_fail.sh Outdated Show resolved Hide resolved
@ddl-alexpanin ddl-alexpanin requested a review from dknupp June 6, 2022 20:26
@ddl-alexpanin
Copy link
Contributor

I think this is great work.

@dknupp can you provide some feedback?

@dknupp
Copy link
Collaborator

dknupp commented Jun 8, 2022 •
edited
Loading

I think this is great work.

@dknupp can you provide some feedback?

Yep -- I will have time today @ddl-alexpanin.

dknupp
dknupp reviewed Jun 8, 2022
View reviewed changes
Copy link
Collaborator

@dknupp dknupp left a comment

Choose a reason for hiding this comment

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

Most of the changes seem mechanical, and look fine to me.

.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
ddl-olsonJD and others added 5 commits June 14, 2022 07:43
@ddl-olsonJD @dknupp
Updated dominodatalab link in PR form
49f14b0 
Co-authored-by: David Knupp <dknupp@users.noreply.github.com>
@ddl-olsonJD @dknupp
Updated checklist in PR form
ce21249 
Co-authored-by: David Knupp <dknupp@users.noreply.github.com>
@ddl-olsonJD
DOM-38006 - updated linting
a60e27d
@ddl-olsonJD
DOM-38006 - updated for mypy tests
49cd4b9
@ddl-olsonJD
DOM-38006 - Updated from feedback
a9f37d1
@ddl-olsonJD
Copy link
Collaborator Author

Screen Shot 2022-06-14 at 07 54 47

@ddl-alexpanin ddl-alexpanin self-requested a review June 14, 2022 18:06
ddl-alexpanin
ddl-alexpanin approved these changes Jun 14, 2022
View reviewed changes
Copy link
Contributor

@ddl-alexpanin ddl-alexpanin left a comment

Choose a reason for hiding this comment

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

LGTM, just take a look at the last 2 comments from me.

@ddl-olsonJD ddl-olsonJD merged commit 2fb3b33 into master Jun 15, 2022
@ddl-olsonJD ddl-olsonJD changed the title [DOM-38006] implement black/flake8 code formatter [DOM-38006] - Implement black/flake8 code formatter Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ddl-alexpanin ddl-alexpanin ddl-alexpanin approved these changes

@dknupp dknupp Awaiting requested review from dknupp

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@ddl-olsonJD @ddl-alexpanin @dknupp