-
Notifications
You must be signed in to change notification settings - Fork 5
/
CONTRIBUTING
233 lines (175 loc) · 9.22 KB
/
CONTRIBUTING
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
<braindump>
Howdy, interested contributor,
My name's Paul, and I've done some of the work designing and implementing this
particular bit of software. Since you're interested in hacking on this code,
I'd like to make a few things clear up front, so you don't end up hating me.
Perhaps you'll still end up hating me, but at least it'll be informed hate.
How it works
============
OK, so this code is actually a pretty novel class-based state machine. The code
may look to some like it's too spread-out (e.g. using "include magic" to drive
the logic), and perhaps that criticism has some merit, but the code's here, and
it's very maintainable.
Why did I rebuild a state machine?
You know, it's a good question. I'd not mind re-considering this, provided that
the new implementation is better.
I don't see some of the other stuff maintainable or readable, so this was mostly
just a rejection of what I found. It's a CS101 topic, so I figured I'd be
able to implement something workable that's clean, clear and maintainable
without too much issue.
If you think this is wrong, email me and let me know why. I'm seriously
interested.
Alright, enough. So, how does it work?
The code processes a stream of bytes, one char at a time.
+-------+ +-------+ +---------+ +--------------+
| bytes | ----> | logic | ----> | machine | ----> | (some state) |
+-------+ +-------+ +---------+ +--------------+
This means (to be clear) under *no* situations I've seen so far will accessing
a state directly from the logic be OK. All state interactions must pass through
the machine, since we must not depend on a state by name in the logic. That
logic is defined in the states themselves.
State UML class diagram:
+---------------------------+
| state |
+---------------------------+
| +leave_state(void) : void |
| +enter_state(void) : void |
| +process(char) : void |
+---------------------------+
`leave_state` is called when process has returned a state other then the current
state. This is a chance for you to finalize the logic without
having sloppy process code.
`enter_state` is called when another state has triggered an entry to this state.
This is where you would init old data structures, etc.
`process` handle a char, and deal with as one should. By raising an
exception (incomplete_process), you can "redirect" the char.
By changing the "xdg_machine_next_state" pointer, you will
trigger the enter / leave functions.
Basic guideliens:
States must only be aware of states that it transitions to (not even where
it's transitioning from), and must only interact with those states by setting
the correct machine pointer.
The exception here is accessing another state's data structure. For instance,
in "value", we know that we must have only come from "key", and we really,
really need "key"'s data structure. In this case, using the symbols exported
in that state's .hh def is allowed. Try to keep this to a minimum, it makes
it very hard to keep some of it up to date, or swap out state implementations.
Current impl:
+-------------------------------------------------------------+
| |
| +------------------------------------+-----+ |
| v | | |
| (entry) ---- "#" ----> (comment) ---- "\n" | |
| | \ ^ \ | |
| | \ \ + ---- "[" ----> (group) ---- "]" ---+ |
| | \ \ \------------------ "\n" --+
| | \ +---------------------------------------- "\n" ------+
| | \ |
| | + ---- [anything else] ----> (key) ---- "=" ----> (value)
| "=" |
| | "\n"
| | |
+-> (invalid) <------------------------------|
Yes, I realize this is *VERY* messy. It's not really something that lends
itsself to ascii art. Just start at "entry" and work from there.
Code Style
==========
My style is fairly straight forward. Just try to make code look like the rest
of the code. I know I sometimes violate my own rules, but the big stuff is
consistent. Try to maintain that, if you can. I'd be more then happy to help
give a bit of help if it's unclear.
|
| int f = 1;
| int longvar = 2;
| std::string bar = "baz";
|
| if ( f )
| baz();
|
| if ( foo == baz ) { /* This is a comment about this unclear bit
| of code, which should help with something */
| baz_foo();
| foo_baz();
| } else {
| groble();
| }
|
"XXX:", "TODO:" and "FIXME:" are all valid task tags. Please use these when you
find something in the code that's not critical, not really worth a bug, but
should be fixed at some point. I tend to use "XXX:".
Submitting code for review
==========================
Guidelines (I will always reject code because one of these is not right)
These are in no particular order.
(1): Write a helpful git changelog entry.
(2): Provide the code under terms compatible with the COPYRIGHT file. I prefer
Expat (since it's the license in the COPYRIGHT), but compatible licenses
are OK too.
(3): Add yourself to AUTHORS on the *FIRST* commit. This is for my protection
and records as well as being a nice way for contributors to see their
name in the project.
(4): Add yourself to the license header of each file you modify, along with
the date and a *valid* email. Do not change the license copyright block,
since the license you've chosen will be Expat compatible (see point 2),
since I'm not re-licensing my code.
(5): If you add a change to the binary's invocation or logic that the user is
exposed to, please note the change in the manual page.
(6): If you see you're out of date, please rebase rather then merge. It's a
few seconds of your time, and it helps keep the history clean.
(7): Use a valid email throughout all the copyrights, etc.
GitHub Pull Request
This method makes it really easy for me to keep track of incoming code in the
same tracker I use to maintain this project. As such, this is the way I
prefer to see code come in. Some advice (purely advice, this won't cause
me to kick code back)
- Consider using branches to help break features and bugfixes into easy
to review and merge blocks. If some pull requests "depend" on others,
that's OK. Just let me know, and I'll do the dependency resolution.
- File bugs early, if you mention you're working on a fix, I'll wait for
you to send your code in. I'd rather have a bug then not.
Format Patch Series
This method is 100% fine, as it's the way we do patches in Fluxbox it's self.
This may also be helpful if you're not using GitHub for reasons such as it's
status as a free software project. Some advice (only advice, I won't reject
code that breaks the following rules)
- Consider attaching format-patches as attachments to a "Hello!" email
where you describe what the series does, why it's something that's
important and any other feedback you have for me. If you decide to GPG
sign any of the mails, this is the one to do it on.
- If you don't care for the above advice, git-send-email works fine too.
Just make sure you have extra-descriptive commit messages.
General diff
I would rather not get a raw diff, but if you must, it's OK. With this,
please include your full name and email (or, rather, *a* full name & working
email) for me to commit the code as. If you'd rather not expose your email,
that's OK. Just let me know. I won't assume the mail address you sent me mail
with is that one that you'd like to make public (perhaps you use tags, public
accounts, etc), so I will ask if you don't include it.
Some additional (and more strictly enforced) tips:
- Be sure the patch applies aginst git head.
- Be sure the patch applies with -p1
- Unified diff, please (try passing -Nuarp to diff)
Some nice to do advice:
- Feel free to add email (or http) style headers to the top of a patch
to serve as a "commit message" (see the exampe[1])
- Most of the time, this *will* be more complex then using git. Think really
hard about using git format-patch. It makes it a little harder on me, and
much harder for you.
[1]:
From: Joe Hacker <[email protected]>
Subject: Adding in "this is a test" to foo1.
Adding in a little message :)
diff -Nuarp bar/foo1 foo/foo1
--- bar/foo1 1970-01-01 00:00:00.000000000 +0000
+++ foo/foo1 2012-03-16 02:07:12.131507334 +0000
@@ -0,0 +1 @@
+This is a test
Thanks for reading, and thanks for considering contributing to fbautostart.
Feel free to get in touch with me by email, irc, jabber, reddit, twitter,
identi.ca, github, launchpad, or any other protocol you might happen to find
me on.
Anyway, I know there's a lot here. Treat it like a ref-doc throughout
contributing to fbautostart.
Hope this helps,
-- Paul
</braindump>