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

[Feature Request] Add change detection to qmk cformat #7884

Closed
2 tasks done
skullydazed opened this issue Jan 13, 2020 · 3 comments · Fixed by #7893
Closed
2 tasks done

[Feature Request] Add change detection to qmk cformat #7884

skullydazed opened this issue Jan 13, 2020 · 3 comments · Fixed by #7893

Comments

@skullydazed
Copy link
Member

skullydazed commented Jan 13, 2020

Currently qmk cformat without any additional arguments runs clang-format against everything. To make it easier and faster to use we should support detecting changes and run only against those files which have been edited.

Feature Request Type

  • Core functionality
  • Alteration (enhancement/optimization) of existing feature(s)

Description

When the user supplies a list of filenames (by running qmk cformat <file1> <file2> <...> <fileN>) the behavior should be unchanged.

When the user runs qmk cformat -a the command should behave exactly as running qmk cformat does today, by running clang-format against all core files.

When the user runs qmk cformat without any arguments we should use the output of git diff --name-only origin/master <core directories> to find the files to format. We should provide a new optional option --base-branch (-b) to specify a branch other than origin/master.

@skullydazed
Copy link
Member Author

The behavior is still a WIP. I've written these down as a starting point but if there's an argument for doing it differently please suggest changes. For example, I'm not entirely fond of the name --master-branch but I'm wary of the implications of having user.branch overriding behavior here.

@noroadsleft
Copy link
Member

I'd suggest --base-branch instead of --master-branch, because base branch is GitHub's term for the target of a pull request. The default branch of a repo isn't always going to be named master. -b alias still works IMO though.

@skullydazed
Copy link
Member Author

I like that name a lot better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants