Skip to content

[Lint] Apply lint #126

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

Merged
merged 64 commits into from
Sep 23, 2022
Merged

[Lint] Apply lint #126

merged 64 commits into from
Sep 23, 2022

Conversation

titaiwangms
Copy link
Contributor

@titaiwangms titaiwangms commented Sep 21, 2022

Should be merged after #120
fixes #125

  1. add auto-formatter lint.sh (feel free to add on others)
  2. flake8, black, and isort formatting is clean now
  3. pylint and mypy is a lot of work to address - need everyone address these step by step (also take them out from style.sh, and make style_optional.sh CI test for them.)
  4. relative path addressed (import module instead of class and function)
  5. We should enable Lint/Enforce as mandatory
  6. Move debuginfo stand alone as it caused circular import.

NOTE: I hope this one can be quickly merged, as there are PRs being merged constantly which means endless merge conflict for this one.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Found 34 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Found 34 potential problems in the proposed changes. Check the Files changed tab for more details.

@titaiwangms titaiwangms reopened this Sep 22, 2022
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Found 34 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Found 33 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Found 33 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Found 33 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Found 33 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Found 33 potential problems in the proposed changes. Check the Files changed tab for more details.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Found 33 potential problems in the proposed changes. Check the Files changed tab for more details.

titaiwangms added a commit that referenced this pull request Sep 23, 2022
This PR basically migrates the lint and CI from ONNX to here.

1. Add lint: black, isort, mypy, and flake8
    - flake8 remains basic setting as main branch.
    - auto-formatter will be introduced in the next PR (#126 )

3. Add pyproject.toml (includes what was in requirements.txt, setup.cfg,
and metadata in setup.py)
- Avoiding parsing documents in setup.py can reduce bugs (version_str
was -0.1-)
    - Put metadata all in one file (pyproject.toml )
- benefit: toml becomes more important in python, check
[here](https://peps.python.org/pep-0680/)
  
4. Add lint CI to align coding style 
- `Lint / Enforce style (pull_request)` CI utilizes style.sh to do the
test
- We can only make `Lint / Enforce style (pull_request)` as required
after a reasonable transition time period
 
6. Continue work (actually apply lint on files) on #126  

fixes #105

Co-authored-by: Ubuntu <titaiwang@titaiwanglinuxcpudev.y3zdd0j2xrqelnmezcgpqgmnte.jx.internal.cloudapp.net>
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Found 33 potential problems in the proposed changes. Check the Files changed tab for more details.

@titaiwangms titaiwangms merged commit 992f554 into microsoft:main Sep 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request topic: documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid relative imports; import only modules
2 participants