From 4dd0c97bbfcad0239449a5ee7f2a4dfaeef1cc0a Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Tue, 10 Mar 2015 09:18:25 -0400 Subject: [PATCH 01/14] Add contributing guidelines GitHub will display this on the pull request page. See https://github.com/blog/1184-contributing-guidelines --- CONTRIBUTING.md | 98 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000000..ee4da9e9230 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,98 @@ +# Welcome! + +This document assumes you have already [set up a development +environment](https://wiki.freenetproject.org/Building_from_source) and have made +a change you want to submit for review. If you'd like an idea for something to +start with: + +* Is there anything you've found annoying or lacking while using Freenet that + you'd like to fix? (There might even be a bug filed already.) +* The [bug tracker](https://bugs.freenetproject.org/my_view_page.php) has tasks. + List only bugs filed against this repository by selecting "Freenet" in the + "Project" drop-down in the upper right. Do any of these look interesting? +* Ask the [development mailing list](https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl) + or join us in [IRC](https://freenetproject.org/irc.html) - `#freenet` on + `chat.freenode.net`. + +# Standards + +Before submitting a pull request, please make sure that modified lines meet the +project [coding standards](https://google-styleguide.googlecode.com/svn/trunk/javaguide.html) +and that the commit messages meet the standards: + +## Commit messages + +From the git commit man page: + +> begin the commit message with a single short (less than 50 character) line +> summarizing the change, followed by a blank line and then a more thorough +> description. The text up to the first blank line in a commit message is +> treated as the commit title... + +Except for proper nouns, only the first character of the first line may be +capitalized. It can start with the class / module the change applies to, +followed by a colon, especially if it helps to explain the change and give it +context. The main part of it is a short imperative description that is at most +72 characters and does not end with a full stop. The second line must be blank +for, among other things, git shortlog to display only the title instead of also +including in the description. If the message contains additional prose +description on the third and subsequent lines, it is wrapped to 72 characters. +Long lines that do not split well, such as URLs, are an exception to this. + +50 characters on the first line is a soft limit. For more discussion see +https://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches#n87 + + [Context:] command given to the codebase + +For example: + + FProxy: update default bookmark editions + + Additional description of what the change does, why it is a good idea, + and alternate solutions decided against, if any, goes here. For some + changes this might not be necessary. + +or + + Reduce hydrocoptic marzul vane side fumbling + + This change is such that starting the summary with an area of change + would not be helpful. The existing marzul vane fittings are prone to + side fumbling at sinusoidal depleneration duty cycles over 80%. This + hyperfastens the vanes to avoid unilateral boloid shedding which + contributes to the side fumbling. Non-reversible tremmy pipes would + have had similar effects but added more weight to the casing. + +Text editors can be configured to assist in formatting messages this way, and +git packages sometimes ship with such configuration. + +Each commit in a pull request should represent a logically distinct subset of +the overall change so that it is easy to review. This means the following is +appropriate for things in development: + + Add more tests for new feature + Fix NPE in new feature + Fix inverted branch condition in new feature + Add tests for new feature + Add exciting new feature + +But this is much easier to review: + + Add tests for new feature + Add exciting new feature + +See the Git documentation on [history rewriting](http://git-scm.com/book/en/v2/Git-Tools-Rewriting-History) +for how to do this. `squash` and `fixup` are the relevant actions. + +# Code review + +Code review reviews code and commits, not their authors. Its goal is producing +code that is readable, correct, and sufficiently efficient. Except for +confusingly inefficient code - so much that it impacts readability - +optimization is to be guided by benchmarks and not what feels faster. + +Breaking API creates an immense amount of work for developers of other projects, +so it is *not* something to be done lightly. If you feel you must make a change +that breaks API and cannot maintain backwards compatibility, please first raise +the issue with the community - the [mailing list](https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl) +and [IRC](https://freenetproject.org/irc.html) are good places to contact us. From 7d29802a5656c8009ede7462a76d78f67fcd41c8 Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Sat, 14 Mar 2015 15:14:37 -0400 Subject: [PATCH 02/14] fixup! Add contributing guidelines --- CONTRIBUTING.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ee4da9e9230..f1e2a481108 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,6 +10,7 @@ start with: * The [bug tracker](https://bugs.freenetproject.org/my_view_page.php) has tasks. List only bugs filed against this repository by selecting "Freenet" in the "Project" drop-down in the upper right. Do any of these look interesting? +* Check the [projects](https://wiki.freenetproject.org/Projects) page. * Ask the [development mailing list](https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl) or join us in [IRC](https://freenetproject.org/irc.html) - `#freenet` on `chat.freenode.net`. @@ -29,15 +30,16 @@ From the git commit man page: > description. The text up to the first blank line in a commit message is > treated as the commit title... -Except for proper nouns, only the first character of the first line may be -capitalized. It can start with the class / module the change applies to, +Except for proper nouns, only the first character of the title may be +capitalized. It may start with the class / module the change applies to, followed by a colon, especially if it helps to explain the change and give it -context. The main part of it is a short imperative description that is at most -72 characters and does not end with a full stop. The second line must be blank -for, among other things, git shortlog to display only the title instead of also -including in the description. If the message contains additional prose -description on the third and subsequent lines, it is wrapped to 72 characters. -Long lines that do not split well, such as URLs, are an exception to this. +context. The main part of it is a short imperative description - a command given +to the codebase. The title is at most 72 characters and does not end with a +period. The second line must be blank for, among other things, git shortlog to +display only the title instead of also including the description. If the message +contains additional prose description on the third and subsequent lines, it is +wrapped to 72 characters. Long lines that do not split well, such as URLs, are +an exception to this. 50 characters on the first line is a soft limit. For more discussion see https://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches#n87 From d482444016577d54e5136deaa233bb0e13f73251 Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Sat, 14 Mar 2015 15:27:13 -0400 Subject: [PATCH 03/14] squash! fixup! Add contributing guidelines Remove turbo encabulator example. It was hopefully amusing, but didn't say much that the previous example didn't address already. --- CONTRIBUTING.md | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f1e2a481108..732749a63fe 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -54,17 +54,6 @@ For example: and alternate solutions decided against, if any, goes here. For some changes this might not be necessary. -or - - Reduce hydrocoptic marzul vane side fumbling - - This change is such that starting the summary with an area of change - would not be helpful. The existing marzul vane fittings are prone to - side fumbling at sinusoidal depleneration duty cycles over 80%. This - hyperfastens the vanes to avoid unilateral boloid shedding which - contributes to the side fumbling. Non-reversible tremmy pipes would - have had similar effects but added more weight to the casing. - Text editors can be configured to assist in formatting messages this way, and git packages sometimes ship with such configuration. From 22d1958f6c9ed32e359b3faada148489b8b85785 Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Sat, 14 Mar 2015 16:15:51 -0400 Subject: [PATCH 04/14] fixup! Add contributing guidelines --- CONTRIBUTING.md | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 732749a63fe..9aa45547032 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,6 +15,18 @@ start with: or join us in [IRC](https://freenetproject.org/irc.html) - `#freenet` on `chat.freenode.net`. +# Code review + +Code review helps improve code quality, ensures that multiple people know the +codebase, and serves as a defense against introducing malicious code. Its goal +is producing code that is readable, correct, and sufficiently efficient. + +Breaking API creates an immense amount of work for developers of other projects, +so it is *not* something to be done lightly. If you feel you must make a change +that breaks API and cannot maintain backwards compatibility, please first raise +the issue with the community - the [mailing list](https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl) +and [IRC](https://freenetproject.org/irc.html) are good places to contact us. + # Standards Before submitting a pull request, please make sure that modified lines meet the @@ -74,16 +86,3 @@ But this is much easier to review: See the Git documentation on [history rewriting](http://git-scm.com/book/en/v2/Git-Tools-Rewriting-History) for how to do this. `squash` and `fixup` are the relevant actions. - -# Code review - -Code review reviews code and commits, not their authors. Its goal is producing -code that is readable, correct, and sufficiently efficient. Except for -confusingly inefficient code - so much that it impacts readability - -optimization is to be guided by benchmarks and not what feels faster. - -Breaking API creates an immense amount of work for developers of other projects, -so it is *not* something to be done lightly. If you feel you must make a change -that breaks API and cannot maintain backwards compatibility, please first raise -the issue with the community - the [mailing list](https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl) -and [IRC](https://freenetproject.org/irc.html) are good places to contact us. From 6540b89a5c7de34cce1a01e5878d93cbbde13249 Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Sat, 14 Mar 2015 16:23:03 -0400 Subject: [PATCH 05/14] fixup! Add contributing guidelines --- CONTRIBUTING.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9aa45547032..88fe9af41cd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,5 +1,3 @@ -# Welcome! - This document assumes you have already [set up a development environment](https://wiki.freenetproject.org/Building_from_source) and have made a change you want to submit for review. If you'd like an idea for something to @@ -18,8 +16,9 @@ start with: # Code review Code review helps improve code quality, ensures that multiple people know the -codebase, and serves as a defense against introducing malicious code. Its goal -is producing code that is readable, correct, and sufficiently efficient. +codebase, serves as a defense against introducing malicious code, and makes it +infeasable to pressure people into contributing malicious code. Its goal is +producing software that is readable, correct, and sufficiently efficient. Breaking API creates an immense amount of work for developers of other projects, so it is *not* something to be done lightly. If you feel you must make a change From 670dc13f93a72fa58a9733d682870ce649287779 Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Sat, 14 Mar 2015 16:23:40 -0400 Subject: [PATCH 06/14] fixup! Add contributing guidelines --- CONTRIBUTING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 88fe9af41cd..e63674f3580 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,3 +1,5 @@ +# Welcome! + This document assumes you have already [set up a development environment](https://wiki.freenetproject.org/Building_from_source) and have made a change you want to submit for review. If you'd like an idea for something to From 4b57b7bfcdf124d682f1fa549fd4448dc9ed1174 Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Sat, 14 Mar 2015 16:31:17 -0400 Subject: [PATCH 07/14] fixup! Add contributing guidelines --- CONTRIBUTING.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e63674f3580..e8756a4e4ca 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -54,8 +54,7 @@ contains additional prose description on the third and subsequent lines, it is wrapped to 72 characters. Long lines that do not split well, such as URLs, are an exception to this. -50 characters on the first line is a soft limit. For more discussion see -https://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches#n87 +For more discussion see the [git patch submission documentation](https://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches#n87) [Context:] command given to the codebase From 769ef96a481ade558df78768246e8e33723332de Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Sat, 14 Mar 2015 16:34:56 -0400 Subject: [PATCH 08/14] fixup! Add contributing guidelines --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e8756a4e4ca..55c2fac294d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -54,7 +54,7 @@ contains additional prose description on the third and subsequent lines, it is wrapped to 72 characters. Long lines that do not split well, such as URLs, are an exception to this. -For more discussion see the [git patch submission documentation](https://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches#n87) +For more discussion see the [git patch submission documentation](https://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches#n87). [Context:] command given to the codebase From f86f48beed8a70c0d2618e03b235f9c9d32f43de Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Fri, 20 Mar 2015 00:02:56 -0400 Subject: [PATCH 09/14] fixup! Add contributing guidelines --- CONTRIBUTING.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 55c2fac294d..0f4f65a660a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -17,6 +17,14 @@ start with: # Code review +Once you have a change ready for review, please submit it as a [pull +request](https://help.github.com/articles/using-pull-requests/#initiating-the-pull-request) +against the `next` branch. It's usually a good idea to work on a topic / feature +branch specific to the pull request that starts at `next`. Once you have +submitted a pull request, people will start reviewing it and providing feedback. +If you don't hear anything for a while feel free to bring it to the attention of +people in IRC - we may have missed it. + Code review helps improve code quality, ensures that multiple people know the codebase, serves as a defense against introducing malicious code, and makes it infeasable to pressure people into contributing malicious code. Its goal is From a2ccd1686363a80412ad3c2736f68351e7ddc0d3 Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Wed, 25 Mar 2015 00:56:18 -0400 Subject: [PATCH 10/14] fixup! Add contributing guidelines --- CONTRIBUTING.md | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0f4f65a660a..ae2a7a5b25b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,10 +20,12 @@ start with: Once you have a change ready for review, please submit it as a [pull request](https://help.github.com/articles/using-pull-requests/#initiating-the-pull-request) against the `next` branch. It's usually a good idea to work on a topic / feature -branch specific to the pull request that starts at `next`. Once you have +branch that starts at `next` and is specific to the pull request. Once you have submitted a pull request, people will start reviewing it and providing feedback. If you don't hear anything for a while feel free to bring it to the attention of -people in IRC - we may have missed it. +people in [IRC](https://freenetproject.org/irc.html) or the [mailing +list](https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl) - we may +have missed it. Code review helps improve code quality, ensures that multiple people know the codebase, serves as a defense against introducing malicious code, and makes it @@ -52,15 +54,13 @@ From the git commit man page: > treated as the commit title... Except for proper nouns, only the first character of the title may be -capitalized. It may start with the class / module the change applies to, -followed by a colon, especially if it helps to explain the change and give it -context. The main part of it is a short imperative description - a command given -to the codebase. The title is at most 72 characters and does not end with a -period. The second line must be blank for, among other things, git shortlog to -display only the title instead of also including the description. If the message -contains additional prose description on the third and subsequent lines, it is -wrapped to 72 characters. Long lines that do not split well, such as URLs, are -an exception to this. +capitalized. It may start with the class / module the change applies to +followed by a colon if it helps to explain the change and give it context. The +main part of it is a short imperative description - a command given to the +codebase. The title is at most 72 characters and does not end with a period. The +second line is blank. If the message contains additional prose description on +the third and subsequent lines, it is wrapped to 72 characters. Long lines that +do not split well, such as URLs or stack traces, are an exception to this. For more discussion see the [git patch submission documentation](https://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches#n87). @@ -86,11 +86,14 @@ appropriate for things in development: Fix inverted branch condition in new feature Add tests for new feature Add exciting new feature + Add support needed by new feature But this is much easier to review: Add tests for new feature Add exciting new feature + Add support needed by new feature See the Git documentation on [history rewriting](http://git-scm.com/book/en/v2/Git-Tools-Rewriting-History) -for how to do this. `squash` and `fixup` are the relevant actions. +for how to do this. `squash` and `fixup` are the relevant actions. `git commit` +has `--fixup` and `--squash` for use with `rebase -i --autosquash`. From 4f02f7e9f279ca7d7ba96ee04eebe7bca129db9e Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Wed, 25 Mar 2015 01:02:47 -0400 Subject: [PATCH 11/14] fixup! Add contributing guidelines --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ae2a7a5b25b..c0908ee6e0b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -68,7 +68,7 @@ For more discussion see the [git patch submission documentation](https://git.ker For example: - FProxy: update default bookmark editions + Update default bookmark editions Additional description of what the change does, why it is a good idea, and alternate solutions decided against, if any, goes here. For some From 05cd2d1f08ea14c9a99d2cc5b1ae6bbeaacc06e3 Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Wed, 25 Mar 2015 01:05:17 -0400 Subject: [PATCH 12/14] fixup! Add contributing guidelines --- CONTRIBUTING.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c0908ee6e0b..b96126f9cc6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -62,8 +62,6 @@ second line is blank. If the message contains additional prose description on the third and subsequent lines, it is wrapped to 72 characters. Long lines that do not split well, such as URLs or stack traces, are an exception to this. -For more discussion see the [git patch submission documentation](https://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches#n87). - [Context:] command given to the codebase For example: @@ -74,9 +72,13 @@ For example: and alternate solutions decided against, if any, goes here. For some changes this might not be necessary. +For more discussion see the [git patch submission documentation](https://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches#n87). + Text editors can be configured to assist in formatting messages this way, and git packages sometimes ship with such configuration. +## Commit separation + Each commit in a pull request should represent a logically distinct subset of the overall change so that it is easy to review. This means the following is appropriate for things in development: From ad3bbcdb6f2fbcea9d2988cae58095c1e3cb8f34 Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Wed, 24 Jun 2015 08:00:06 -0400 Subject: [PATCH 13/14] fixup! Add contributing guidelines --- CONTRIBUTING.md | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b96126f9cc6..1e23673ab18 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -46,31 +46,24 @@ and that the commit messages meet the standards: ## Commit messages -From the git commit man page: +The first line is the title: -> begin the commit message with a single short (less than 50 character) line -> summarizing the change, followed by a blank line and then a more thorough -> description. The text up to the first blank line in a commit message is -> treated as the commit title... +* Write it as a command to the codebase +* Limit the length to 72 characters +* Do not put a period at the end +* (optional) Separate context with a colon -Except for proper nouns, only the first character of the title may be -capitalized. It may start with the class / module the change applies to -followed by a colon if it helps to explain the change and give it context. The -main part of it is a short imperative description - a command given to the -codebase. The title is at most 72 characters and does not end with a period. The -second line is blank. If the message contains additional prose description on -the third and subsequent lines, it is wrapped to 72 characters. Long lines that -do not split well, such as URLs or stack traces, are an exception to this. - - [Context:] command given to the codebase +The second line is blank. If the message contains additional prose description +on the third and subsequent lines, it is wrapped to 72 characters. Long lines +that do not split well, such as URLs or stack traces, are an exception to this. For example: Update default bookmark editions Additional description of what the change does, why it is a good idea, - and alternate solutions decided against, if any, goes here. For some - changes this might not be necessary. + and any alternate solutions that seem more obvious and were decided + against goes here. For some changes this might not be necessary. For more discussion see the [git patch submission documentation](https://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches#n87). From ac045d9dc7092e301865e21f2f2a8a22fdda9cef Mon Sep 17 00:00:00 2001 From: Steve Dougherty Date: Wed, 24 Jun 2015 08:00:59 -0400 Subject: [PATCH 14/14] squash! Add contributing guidelines We could not agree that this was a viable way to do things. --- CONTRIBUTING.md | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1e23673ab18..2e66216d912 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -69,26 +69,3 @@ For more discussion see the [git patch submission documentation](https://git.ker Text editors can be configured to assist in formatting messages this way, and git packages sometimes ship with such configuration. - -## Commit separation - -Each commit in a pull request should represent a logically distinct subset of -the overall change so that it is easy to review. This means the following is -appropriate for things in development: - - Add more tests for new feature - Fix NPE in new feature - Fix inverted branch condition in new feature - Add tests for new feature - Add exciting new feature - Add support needed by new feature - -But this is much easier to review: - - Add tests for new feature - Add exciting new feature - Add support needed by new feature - -See the Git documentation on [history rewriting](http://git-scm.com/book/en/v2/Git-Tools-Rewriting-History) -for how to do this. `squash` and `fixup` are the relevant actions. `git commit` -has `--fixup` and `--squash` for use with `rebase -i --autosquash`.