Skip to content

Add Format and Lint to Makefile (#545)#610

Merged
khorshuheng merged 2 commits intofeast-dev:v0.4-branchfrom
khorshuheng:backport-545
Apr 14, 2020
Merged

Add Format and Lint to Makefile (#545)#610
khorshuheng merged 2 commits intofeast-dev:v0.4-branchfrom
khorshuheng:backport-545

Conversation

@khorshuheng
Copy link
Copy Markdown
Collaborator

@khorshuheng khorshuheng commented Apr 8, 2020

Backport #545

@feast-ci-bot
Copy link
Copy Markdown
Collaborator

feast-ci-bot commented Apr 8, 2020

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: khorshuheng
To complete the pull request process, please assign thirteen37
You can assign the PR to them by writing /assign @thirteen37 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ches
ches reviewed Apr 8, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@ches ches left a comment

Choose a reason for hiding this comment

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

Getting consistency on the active branches makes life easier for backporting 👍

/lgtm

sdk/go/protos/feast/core/Store.pb.go Outdated
Copy link
Copy Markdown
Member

@ches ches Apr 8, 2020

Choose a reason for hiding this comment

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

There was a bit of codegen that was stale here I guess. Also a few Go module version updates, I think the Go SDK is kind of dormant so those aren't significant.

@ches
Copy link
Copy Markdown
Member

ches commented Apr 8, 2020

Ah, looks like Google credentials are a problem for the Actions build though.

@woop
Copy link
Copy Markdown
Member

woop commented Apr 9, 2020

Ah, looks like Google credentials are a problem for the Actions build though.

On master I have removed those tests. I know that isn't the ideal situation, but the functionality should still be covered by e2e

https://github.com/gojek/feast/blob/master/sdk/python/tests/test_client.py#L407

Relevant issue: #562

@feast-ci-bot feast-ci-bot removed the lgtm label Apr 14, 2020
@feast-ci-bot
Copy link
Copy Markdown
Collaborator

feast-ci-bot commented Apr 14, 2020

New changes are detected. LGTM label has been removed.

@khorshuheng khorshuheng force-pushed the backport-545 branch 3 times, most recently from c9d5040 to 2933167 Compare April 14, 2020 04:35
@woop @khorshuheng
Add Format and Lint to Makefile (feast-dev#545)
03b96f4 
* Add Make commands for format, lint, flake8, spotless, isort, black, and refactor

* Add mypy test

* Add lint tests to CI

* Fix broken Python test

* Fix broken test for Python

* Add black to dependencies

* Remove Python Protos

* Add automatic local linting

* Update precommit names

* Add black exclusions

* Add tensorflow metadata proto generation

* Ignore tf meta directory

* Add build essentials to install make in CI

* Add exports back to __init__.py

* Add __all__ to export

* Add white space to export

* Add source to export

* Fix python export formatting
@khorshuheng khorshuheng merged commit 3d2afb9 into feast-dev:v0.4-branch Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ches ches ches left review comments

@davidheryanto davidheryanto Awaiting requested review from davidheryanto davidheryanto is a code owner

@pradithya pradithya Awaiting requested review from pradithya pradithya is a code owner

@woop woop Awaiting requested review from woop woop is a code owner

@zhilingc zhilingc Awaiting requested review from zhilingc zhilingc is a code owner

Assignees

@ches ches

Labels

size/XXL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@khorshuheng @feast-ci-bot @ches @woop