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

Code Standards / Linting checked & applied consistently for all langs #526

Closed
ches opened this issue Mar 9, 2020 · 2 comments
Closed

Code Standards / Linting checked & applied consistently for all langs #526

ches opened this issue Mar 9, 2020 · 2 comments

Comments

@ches
Copy link
Member

ches commented Mar 9, 2020

Is your feature request related to a problem? Please describe.

Code standards expectations are by documentation for some project languages, and by automation for others. In cases of the former:

  1. Contributors don't realize they should follow them
  2. Contributors use different versions or settings for tools like formatters
  3. Non-compliant code sometimes gets checked in and merged. When someone later fixes it i.e. by running a formatter, their PR may be inflated with many out-of-scope changes.

Describe the solution you'd like

For all of Go, Java, and Python:

  • Standards are checked by CI, to address issues 1 and 3.
  • Development dependency versions and settings (linting and formatting tools) are managed by build automation, to alleviate issue 2.

Try to fail fast for feedback, but without undue friction on development. For Java for example, we complain on mvn test but not mvn compile (nor test-compile). Perhaps better yet, on CI all the linting could be in one dedicated job / GH status check (though a failure should report all problems, not fail one language, fix and push, fail another language…).

Git hooks could be an optional suggestion.

@woop suggested on #508:

How about we just have make lint or make format at the root of the Feast repo? That would be based on hardcoded configuration that everyone can see, and should ideally cover the whole repo (Java, Go, Python).

Sounds good to me.

Describe alternatives you've considered

Additional context

#487
#508

@woop
Copy link
Member

woop commented Mar 18, 2020

Oops, accidentally closed this issue instead of #508.

@woop
Copy link
Member

woop commented Mar 28, 2020

I'm pretty sure this is sufficiently resolved for now. Happy to reopen though. Closing for now.

@woop woop closed this as completed Mar 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants