Expanded CONTRIBUTING guide
Now includes more notes about specs, line wrapping, writing Git commit messages and more.
This commit is contained in:
parent
5951a6f187
commit
f814ec7170
107
CONTRIBUTING.md
107
CONTRIBUTING.md
|
@ -1,22 +1,57 @@
|
||||||
# Contributing
|
# Contributing
|
||||||
|
|
||||||
Everybody is more than welcome to contribute to Oga, no matter how small the
|
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
|
change. To keep everything running smoothly there are a bunch of guidelines that
|
||||||
should follow. These are as following:
|
one should follow.
|
||||||
|
|
||||||
|
## General
|
||||||
|
|
||||||
* When changing code make sure to write RSpec tests for the changes.
|
* 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
|
* Document code using YARD. At the very least the method arguments and return
|
||||||
value(s) should be documented.
|
value(s) should be documented.
|
||||||
* Wrap lines at 80 characters per line.
|
* Use `raise` for raising errors instead of `fail`. You're raising errors after
|
||||||
* Git commits should have a <= 50 character summary, optionally followed by a
|
all, not failing them.
|
||||||
blank line and a more in depth description of 72 characters per line.
|
|
||||||
|
## 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 <tr> element should be closed first
|
||||||
|
when bumping into any element other than <td> or <th>. 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
|
||||||
|
<td> and <th> 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
|
## 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. Most modern editors can eaisly handle
|
||||||
* Hard wrap lines at 80 characters per line.
|
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]
|
To make this process easier Oga comes with an [EditorConfig][editorconfig]
|
||||||
configuration file. If your editor supports this it will automatically apply
|
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
|
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
|
## Continuous Integration
|
||||||
|
|
||||||
Two continuous integration services are used to ensure the tests of Oga pass
|
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()
|
org.foo.bar.baz.DoSomething()
|
||||||
end
|
end
|
||||||
|
|
||||||
For loading files in Oga itself `require_relative` should be used, _don't_
|
For loading files in Oga itself `require` should be used. _Don't_
|
||||||
modify `$LOAD_PATH`.
|
modify `$LOAD_PATH`, instead run any scripts using `ruby -I lib`.
|
||||||
|
|
||||||
## Contact
|
## Contact
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue