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

Align cformat rules with current CI implementation #7936

Merged
merged 2 commits into from
Jan 22, 2020

Conversation

zvecr
Copy link
Member

@zvecr zvecr commented Jan 19, 2020

Description

Noticed while fixing #7934, quantum/template/* gets included in clang-format, which breaks the tokens #define MANUFACTURER % YOUR_NAME %.

Types of Changes

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

Checklist

  • My code follows the code style of this project.
  • 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).

@zvecr zvecr added the bug label Jan 19, 2020
@zvecr zvecr requested review from skullydazed and a team January 19, 2020 00:10
@zvecr zvecr added cli qmk cli command python labels Jan 19, 2020
@@ -26,7 +26,8 @@ def cformat(cli):
else:
for dir in ['drivers', 'quantum', 'tests', 'tmk_core']:
for dirpath, dirnames, filenames in os.walk(dir):
if 'tmk_core/protocol/usb_hid' in dirpath:
ignores = ['tmk_core/protocol/usb_hid', 'quantum/template']
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ignores = ['tmk_core/protocol/usb_hid', 'quantum/template']

I'd move this line out of the loops, right after else and would use Path objects:
ignores = [Path('tmk_core/protocol/usb_hid'), Path('quantum/template')]

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved.

Not sure I like the mixture of Path and strings, and would prefer to defer it to another round of #7872.

Copy link
Member

Choose a reason for hiding this comment

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

Well, you can use os.path, something like if os.path.join(dirpath, dn) in ignores.

But it's just me thinking about the code, you original code worked enough, with 7b97f2d it works better, so I'm cool with it.

Comment on lines 30 to 31
if any(i in dirpath for i in ignores):
continue
Copy link
Member

@Erovia Erovia Jan 19, 2020

Choose a reason for hiding this comment

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

Instead of skipping every dir/file individually, this will remove the ignorable(?) directory from the list and os.walk will not descend into it.

   for dn in dirnames: 
       if Path(dirpath) / dn in ignores: 
           dirnames.remove(dn) 

@drashna drashna merged commit 8e500c3 into qmk:master Jan 22, 2020
noroadsleft pushed a commit to noroadsleft/qmk_firmware that referenced this pull request Jan 24, 2020
* Align cformat rules with current CI implementation

* Optimise file walking
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request Feb 12, 2020
* Align cformat rules with current CI implementation

* Optimise file walking
fdidron pushed a commit to zsa/qmk_firmware that referenced this pull request Feb 14, 2020
* Align cformat rules with current CI implementation

* Optimise file walking
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Feb 21, 2020
* Align cformat rules with current CI implementation

* Optimise file walking
fdidron pushed a commit to zsa/qmk_firmware that referenced this pull request Feb 26, 2020
* Align cformat rules with current CI implementation

* Optimise file walking
c0psrul3 pushed a commit to c0psrul3/qmk_firmware that referenced this pull request Mar 23, 2020
* Align cformat rules with current CI implementation

* Optimise file walking
kylekuj pushed a commit to kylekuj/qmk_firmware that referenced this pull request Apr 21, 2020
* Align cformat rules with current CI implementation

* Optimise file walking
@zvecr zvecr deleted the feature/cformat_templates branch April 28, 2020 01:09
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* Align cformat rules with current CI implementation

* Optimise file walking
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cli qmk cli command python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants