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

Cleanup & Lint Code #35

Merged
merged 261 commits into from
Oct 29, 2023
Merged

Cleanup & Lint Code #35

merged 261 commits into from
Oct 29, 2023

Conversation

Marshall-Hallenbeck
Copy link
Collaborator

@Marshall-Hallenbeck Marshall-Hallenbeck commented Sep 22, 2023

  • Add Ruff configuration (version pinned due to discrepancies on GitHub runner versioning)
  • Create linter workflow to run Ruff on push & pull request
  • Remove encoding specification from files (unnecessary in Py3)
  • Update strings to be more descriptive, remove typos, and be properly capitalized
  • Change additionally remaining .format() and % old string interpolation to f-string usage (partially FLY)
  • Fix blank Except statements and unnecessary parenthesis in Excepts (partially RSE)
  • Update exception handling for some circumstances where another except was thrown, causing unnecessary output
  • Remove unused imports
  • Fix poorly and non-pythonic variable/function/class names
  • Fix additional single/double quote usage (Q)
  • Add docstrings to some functions and fix docstrings for others
  • Fix usages of mutable function defaults (see B006, mutable-argument-default in Ruff)
  • Properly inform user if file they specified doesn't exist for several modules
  • Fix usages of comprehension and list/dict initialization via Ruff (C4)
  • Remove unnecessary str-concat (ISC)
  • Fix unnecessary pass statements and unnecessary creation of additional variables before return (PIE)
  • Fix some pytest style (PT)
  • Fix return statements returning None (unnecessary) (RET)
  • Add --poetry option for e2e tests, so all commands are prepended with poetry run
  • Fix ftp class name (got changed to "Ftp" by accident)
  • Simplify lots of code (SIM)
  • Fix tests using a password file to properly reference said file (was missing data/)
  • Remove commented out code (ERA)
  • Import and call sys.exit() instead of just exit() (PL)
  • Fix some try except outside loops (PERF203); additional ones are ignored for now
  • Implement list and dict comprehension where possible and preferred (PERF401)
  • Fix some spaces before inline comments (E261)
  • Modernize some code via Refurb (FURB)
  • Fix bug in add-computer module where improper access was being requested, causing an exception
  • Fix bug in add-computer module where module was not exiting if the computer already exists
  • Add in e2e tests for several missing modules

@Marshall-Hallenbeck Marshall-Hallenbeck added this to the v1.1.0 milestone Sep 22, 2023
@Marshall-Hallenbeck Marshall-Hallenbeck self-assigned this Sep 22, 2023
@NeffIsBack NeffIsBack added the enhancement New feature or request label Sep 22, 2023
@NeffIsBack
Copy link
Contributor

NeffIsBack commented Oct 26, 2023

@Marshall-Hallenbeck probably found the reason why github shows some files as completely redone:

will leave it as it, now everything seems to be in CRLF.

EDIT:
VSCode tells me on the dev branch the files are in CRLF as well, not sure what is happening here...

@NeffIsBack
Copy link
Contributor

@Marshall-Hallenbeck probably found the reason why github shows some files as completely redone:

will leave it as it, now everything seems to be in CRLF

We need to be careful with merge conflicts in these files tho. For example pyproject.toml file is missing pyreadline now and this won't be highlighted now

@Marshall-Hallenbeck
Copy link
Collaborator Author

@NeffIsBack tests seem good (other than missing kerberos for WinRM, but that's not related to this PR), and linting passes. I'm good with this so if you are too we should merge it and cut a v1.1.0 release along with your other stuff branched off this.

nxc/connection.py Outdated Show resolved Hide resolved
@NeffIsBack
Copy link
Contributor

@Marshall-Hallenbeck gonna test basic commands tomorrow just to make sure and then this is ready to merge for me

@NeffIsBack NeffIsBack added reviewed code Label for when a static code review was done tested labels Oct 27, 2023
Copy link
Contributor

@NeffIsBack NeffIsBack left a comment

Choose a reason for hiding this comment

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

LGTM🎉

@Marshall-Hallenbeck Marshall-Hallenbeck merged commit dee5e28 into develop Oct 29, 2023
2 checks passed
@Marshall-Hallenbeck Marshall-Hallenbeck deleted the marshall_cleanup branch October 29, 2023 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request reviewed code Label for when a static code review was done tested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Ruff Linter
3 participants