This is an attempt to fix#133.
Previously, we just clobbered the recent GUIDs with the lastest response
every single time, assuming that Atom/RSS feeds would consistently return the
same items. This appears to not be the case. In the wild, the number of items
returned on a single request can vary (sometimes even being 1 or 2 when usually
it is 50!).
This patch alters *how many* and *which* GUIDs we keep between requests, in an
attempt to prevent sending old news for buggy RSS feeds.
In the wild it looks like some RSS feeds will occasionally return 0 items
to requests *but not return an error*. This previously meant we would clobber
our knowledge of recent GUIDs with the empty set. This meant that the next
successful poll would resend the **entire** RSS feed.
Now broken down into:
- `labels` : Labelling/Unlabelling issues/PRs
- `milestones` : Milestoning/Demilestoning issues/PRs
- `assign` : Assigning/Unassigning issues/PRs
This is broken down in the guts of parsing the webhook event such that it
appears to be a unique `X-GitHub-Event` type.
In the spirit of "if you have to do something 3 times then factor it
out", make a testutils package to put all the `RoundTrip` boilerplate.
I don't overly like having test packages, especially mixed in with
code, but I don't see a nicer way of doing this without ending up
with a sprawling mess of copypasta'd test boilerplate which will
be an absolute nightmare to maintain.
I think this is the lesser of two evils.
We exclusively view logs using `less` and `tail`. These do not read JSON well.
Logging as JSON makes it a PITA to read logs and debug problems. We do not
appear to make use of JSON logging, and have no good terminal-based
structured log viewer either.
As a result, I've now removed JSON logging from this project and replaced it
with the standard `TextFormatter` (colors off). This still logs in a structured
way:
```
time="2016-11-18 16:25:46.787373" level=info msg="Got filter ID" filter=717 syncing=1 user_id="@goneb:localhost"
time="2016-11-18 16:25:46.787525" level=info msg="Starting sync" next_batch="s26928_287972_2_1029_26_1_2" syncing=1 user_id="@goneb:localhost"
```
So we can still analyse logs sanely at a later date should we need to. This
feels like the best compromise here between pragmatism and ideals.
The example from the docs was very clearly with spaces after hours and minutes:
```
"committed_at": "2011-11-11T11: 11: 11Z",
```
But in fact those spaces do not exist in the wild. Great.
We can't just use a public field because the caller may clobber it. If they
do clobber it, we've then lost the ability to set it back to what it was because
we don't store another copy anywhere. This patch fixes it by keeping a private
ref and always clobbering on Register().
This is horrible. We need to separate internal-storage and external-fields
better.