Skip to content

[CLI] Refactor open into pathlib.Path.open #23105

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

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

elpekenin
Copy link
Contributor

@elpekenin elpekenin commented Feb 18, 2024

Description

As per title.
While doing a PR i noticed a few open(Path, mode) lines which felt a bit off (non-idiomatic), and after looking for open on lib/python i've noticed that code does indeed use this method on most places, so refactored the ones that don't for consistency.

Notes:

  • I haven't tested any of the changes yet, will try and do later today. Thus, draft
  • I was unsure whether a value was Path or str looking at the code and added a couple type hints on my way to discovery
  • On code looking for comments (license headers), shouldn't strip be used instead on rstrip? ie: remove leading -and not trailing- spaces that would make the str.startswith not detect an existing comment

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@github-actions github-actions bot added python cli qmk cli command labels Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli qmk cli command python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant