From f814ec7170d5f62b4e5dd1dde01163e8418f4ed9 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Sun, 7 Jun 2015 17:22:44 +0200 Subject: [PATCH] Expanded CONTRIBUTING guide Now includes more notes about specs, line wrapping, writing Git commit messages and more. --- CONTRIBUTING.md | 107 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 97 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 97dedd2..193a6fb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,22 +1,57 @@ # Contributing Everybody is more than welcome to contribute to Oga, no matter how small the -change. To keep everything running smoothly there are a few guidelines that one -should follow. These are as following: +change. To keep everything running smoothly there are a bunch of guidelines that +one should follow. + +## General * When changing code make sure to write RSpec tests for the changes. * Document code using YARD. At the very least the method arguments and return value(s) should be documented. -* Wrap lines at 80 characters per line. -* Git commits should have a <= 50 character summary, optionally followed by a - blank line and a more in depth description of 72 characters per line. +* Use `raise` for raising errors instead of `fail`. You're raising errors after + all, not failing them. + +## Git + +Git commits should have a <= 50 character summary, optionally followed by a +blank line and a more in depth description of 72 characters per line. For +example: + + Use blacklists/whitelists for HTML closing rules + + This allows for more fine grained control over when to close certain + elements. For example, an unclosed element should be closed first + when bumping into any element other than or . Using the old + NodeNameSet this would mean having to list every possible HTML element + out there. Using this new setup one can just create a whitelist of the + and elements. + +Please, _please_ write meaningful commit messages. Writing a good commit +messages is _just_ as important as writing good code. If you're having trouble +writing a commit message you should try to break the commits up into smaller +chunks. You can do so using a `git rebase`. ## Editor Setup -Whatever editor you use doesn't matter as long as it can do two things: +Use spaces for indentation, tabs are not accepted. The usage of spaces ensures +the indentation is identical no matter what program or system is used to view +the source code. -* Use spaces for indentation. -* Hard wrap lines at 80 characters per line. +Hard wrap lines at 80 characters per line. Most modern editors can eaisly handle +this, if not you should get a better editor. For example, in Vim you can select +text in visual mode (using `v`) and press `gq` to automatically re-wrap the +selected text. + +It's OK if a line is a few characters longer than 80 but _please_ keep it as +close to 80 characters as possible. Typically I do this when wrapping the line +results in several extra lines without it being much more readable. + +I often have multiple windows vertically next to each other and 80 characters +per line is the only setup that lets me do so, even on smaller screen +resolutions. For example, my typical setup is 1 file browser and two vertical +windows. Using 80 characters per line ensures all code fits in that space along +with some slight padding to make reading more pleasant. To make this process easier Oga comes with an [EditorConfig][editorconfig] configuration file. If your editor supports this it will automatically apply @@ -54,6 +89,58 @@ benchmark is just a matter of running a Ruby script, for example: ruby benchmark/xml/parser/parser_bench.rb +## Tests + +Tests are written using RSpec and use the "should" syntax instead of the +"expect" syntax (for as long as RSpec keeps supporting this). This means that +assertions are written as following: + + some_object.should == some_value + +instead of this: + + expect(some_object).to eq(some_value) + +Specification blocks should be written using `it`, grouping should be done using +`describe`. Specification descriptions should be meaningful and human friendly +English. For example: + + describe Oga::XML::Entities do + describe 'decode' do + it 'decodes < into <' do + # ... + end + end + end + +Typically the top-level `describe` block is used to describe a method name. In +such a case use `describe 'foo'` for class methods and `describe '#foo'` for +instance methods. + +Do not use `let` for creating data re-used between specifications, instead use +a `before` block that sets an instance variable. In other words, use this: + + before do + @number = 10 + end + +instead of this: + + let(:number) { 10 } + +Instance variables stand out much more and they don't require one to also +understand what exactly `let` does which in turn simplifies the process of +reading and writing specifications. + +Whenever adding new specifications please keep them in the existing style unless +I indicate otherwise. There's nothing more annoying than inconsistent +specifications. + +If you insist on changing the structure/style of specifications please open an +issue and ask about it _before_ making any changes. I am very picky about how I +want things and it would be a shame for somebody to spend hours working on +something that isn't going to be merged in any way. + ## Continuous Integration Two continuous integration services are used to ensure the tests of Oga pass @@ -122,8 +209,8 @@ conditional. For example: org.foo.bar.baz.DoSomething() end -For loading files in Oga itself `require_relative` should be used, _don't_ -modify `$LOAD_PATH`. +For loading files in Oga itself `require` should be used. _Don't_ +modify `$LOAD_PATH`, instead run any scripts using `ruby -I lib`. ## Contact