semantic-kernel icon indicating copy to clipboard operation
semantic-kernel copied to clipboard
verified publisher icon microsoft

Add Lint Workflow

Open AdityaGudimella opened this issue 2 years ago • 6 comments

Motivation and Context

This PR adds a Lint Github workflow so that code style rules can be enforced for PRs.

  1. This forms a baseline workflow that can be used as a template for future workflows that will be added.
  2. It helps enforce code style rules. Currently it only checks PyCodeStyle and PyFlakes. Others will be added in the future.
  3. Contributes to automated testing
  4. Does not fix any open issue.

Description

Added Ruff as a dev dependency. Added Github workflow which runs Ruff.

Contribution Checklist

  • [x] The code builds clean without any errors or warnings
  • [x] The PR follows SK Contribution Guidelines (https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
  • [x] All unit tests pass, and I have added new tests where possible
  • [x] I didn't break anyone :smile:

AdityaGudimella avatar Mar 20 '23 18:03 AdityaGudimella

This would be awesome to have! Looks great!

(@alexchaomander and @dluc how do we want to do reviews on the Python side of things? Are there designated folks from the main SK team that are going to sign off, or are those of us working on SK Python going to be the ones to approve? Could do both? Thoughts?)

jjhenkel avatar Mar 20 '23 19:03 jjhenkel

@jjhenkel I'm all for merging this, could you test and approve?

Are there designated folks from the main SK team that are going to sign off, or are those of us working on SK Python going to be the ones to approve? Could do both? Thoughts?

no one designated, but having your approvals would be a great plus at least as a sign that everything is in order. While we're in preview we can move fast. We love small PRs, please help with that :-) I'll help with design and sync across languages.

dluc avatar Mar 21 '23 06:03 dluc

@AdityaGudimella I was able to run this locally via act to test things out, but only after a few small tweaks to the workflow. I'm not sure if they're necessary, or just required for local testing. If it wont hurt the workflow, it would be nice to have it be testable locally via a tool like act!

jjhenkel avatar Mar 21 '23 17:03 jjhenkel

@jjhenkel There are a couple of ways to do what you want. Please tell me which one you prefer.

  1. Use a separate tool to run tests locally.
  2. Use a single tool (like nox) that can be used locally and will also be used in the github actions.
  3. Make a simple cli that runs the different tests and use it both locally and in the workflow.

I personally prefer Option 2 because it keeps things consistent between the workflows and local development. Also what is act? Could you provide a link to it please?

AdityaGudimella avatar Mar 23 '23 03:03 AdityaGudimella

@AdityaGudimella when I refer to testing here I just mean testing the workflow yml you wrote (not tests for the code). I was using this tool: https://github.com/nektos/act to verify things locally (and, to get things to run, I needed to make those small changes)

jjhenkel avatar Mar 23 '23 04:03 jjhenkel

This is super cool and super useful! Let me add support to this right away!

AdityaGudimella avatar Mar 23 '23 16:03 AdityaGudimella