Making a default file set for threadfin reconcile

[radnonymous]

#1
When running reconcile method on main and chase accounts with default config, yields "Register is empty of journal entries."
This seems like an unusual result. If anything is wrong, I expect it is with the ccsample.beancount file.

[kfogel]

It would be good to give an overview here of what this PR does. The title helps, but a short paragraph saying something like "add a default sample config file and sample data files" or whatever would put the reader in the right frame of mind for reviewing the diff.

[kfogel]

This commit needs a real commit message :-).

We can tell you've updated reconcile.py just from looking at the commit metadata itself, which lists the files changed. What the reader wants to know is why and (at an overview level) how. Point 2 in this comment might be useful to read.

[kfogel]

Same with this commit.

[kfogel]

Re this commit: I recommend just using present-tense active voice in commit messages.

Separately from that, there's an issue here that your commit message is currently ambiguous (partly because it uses the past tense both for what happened in this commit and for things that happened in the past). From reading it, I can't tell whether something was just renamed in this commit -- that was my first guess, but now I believe it's wrong -- or whether this commit removes a folder because there is now (thanks to some previous commit) another folder in existence that makes the just-removed folder obsolete.

E.g., Instead of " Removed redundant folder. Was switched to one with a more accurate name (generic_samples)", try something like:

Remove redundant folder

Commit BLAHBLAH_SOME_COMMIT_ID added the 'generic_samples' folder,
which replaces "config_samples", so remove the latter.

However, the higher-level question is why not do the rename all occur in one commit? That is, if you're changing the name of a folder, that should all happen in one commit, so that both sides of the action can be seen at once. The rename should be treated as an atomic operation.

[kfogel]

Why did you switch to hyphen instead of underscore in the middle of the variable?

[radnonymous]

Because "OTS-BOOKKEEPPING" is the name of the repository. It is not two terms, rather one term specifically with that character connecting them. I thought it would be could to be consistent with the name

[kfogel]

Ah, gotcha. However, the repository is also lower case... meanwhile, there's an environment variable whose name is "OTS_BOOKKEEPING", which is probably where the user is going to look anyway to figure out where their ${OTS_BOOKEEPING} dir is. I would just use underscore here; it corresponds to the env variable, and it won't confuse the user.

Your response made sense, btw! It's a judgement call. It's also not the most important thing in the world, of course. But IMHO just using underscores consistently here will be slightly easier for the user. You can also put a comment above that line explaining what "OTS_BOOKKEEPING" refers to, if YAML supports comments (I don't know if it does).

[kfogel]

I think it might be better to require that the user pass a --config option, since in actual use they will always have to. Having the program's invocation method be different for the sample data than for real data partially defeats the purpose of having sample data in the first place :-).

The way to tell the user how to try things out with sample data is to document it in the README, or wherever the usage instructions are. So, relatedly to my comment above about default behavior, this PR needs to include any appropriate documentation updates to match the code changes you've made (and that's a rule in general for PRs).