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

Update Contrib doc #4068

Merged
merged 6 commits into from
Oct 7, 2018
Merged

Update Contrib doc #4068

merged 6 commits into from
Oct 7, 2018

Conversation

drashna
Copy link
Member

@drashna drashna commented Oct 4, 2018

No description provided.

docs/contributing.md Show resolved Hide resolved
};

int foo(void) {
if (cond1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe make this some_condition, cond1 seems oddly specific (I know I wrote this lol).

@@ -57,7 +57,7 @@ Never made an open source contribution before? Wondering how contributions work
Most of our style is pretty easy to pick up on, but right now it's not entirely consistent. You should match the style of the code surrounding your change, but if that code is inconsistent or unclear use the following guidelines:

* We indent using two spaces (soft tabs)
* We use One True Brace Style
* We use [One True Brace Style](http://google.github.io/styleguide/javaguide.html) (java specific things should be ignored)
Copy link
Contributor

Choose a reason for hiding this comment

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

With the example, I don't think this link is needed. Adding Java into the mix might just make things confusing.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we need the word modified here?

  • We use a modified One True Brace Style

@@ -57,7 +57,7 @@ Never made an open source contribution before? Wondering how contributions work
Most of our style is pretty easy to pick up on, but right now it's not entirely consistent. You should match the style of the code surrounding your change, but if that code is inconsistent or unclear use the following guidelines:

* We indent using two spaces (soft tabs)
* We use One True Brace Style
* We use a modified One True Brace Style
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say it's a One True Brace Style variant rather than a modification. But I'm nitpicking here, feel free to ignore me.

@jackhumbert jackhumbert merged commit 7458ac9 into qmk:master Oct 7, 2018
@jackhumbert
Copy link
Member

Awesome - thanks!

@drashna drashna deleted the contrib_doc branch October 7, 2018 00:44
akatrevorjay added a commit to akatrevorjay/qmk_firmware that referenced this pull request Oct 8, 2018
* 'master' of git://github.com/qmk/qmk_firmware:
  Keymap: Adds TheVan RoadKit Gamepad Layout (qmk#4090)
  Update and clarify Unicode documentation (qmk#4065)
  Update Contrib doc (qmk#4068)
  Add non-US Hash and Backslash to AutoShift handling
  Keymap: Update keyboards/planck/keymaps/vifon (qmk#4084)
zer09 pushed a commit to zer09/qmk_firmware that referenced this pull request Oct 20, 2018
* Add link for style

* ignore java

* Add example and update link?

* Minor fixes

* Change 1TBS text

* comments
rseymour pushed a commit to rseymour/qmk_firmware that referenced this pull request Mar 13, 2019
* Add link for style

* ignore java

* Add example and update link?

* Minor fixes

* Change 1TBS text

* comments
yamad pushed a commit to yamad/qmk_firmware that referenced this pull request Apr 10, 2019
* Add link for style

* ignore java

* Add example and update link?

* Minor fixes

* Change 1TBS text

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

Successfully merging this pull request may close these issues.

4 participants