Refactor options storage

[kbrock]

This is #600 rebased and with commits split up/flattened.

• All writers are removed. (you shouldn't change ancestry options on-the-fly, that'll lead to unexpected behavior)
• All instance accessors are removed.
• Even though we do not mention ancestry_column or ancestry_delimiter accessors in the docs, this will be a breaking change for developers, so lets bump a major version.

@kshnurov Please let me know if you feel this does not accurately reflect your previous PR I was having trouble splitting apart your commits and still giving you attribution

[kbrock]

@kshnurov Were you thinking the ancestry_options hash would contain all the configuration options with defaults assigned or just the options that we need elsewhere? e.g.: we could remove :orphan_strategy since it is no longer used outside has_ancestry

[kshnurov]

I can reopen and rebase #600 of you're up to merge it. I don't understand the goal of splitting commits thought.

@kshnurov Were you thinking the ancestry_options hash would contain all the configuration options with defaults assigned or just the options that we need elsewhere? e.g.: we could remove :orphan_strategy since it is no longer used outside has_ancestry

All of them, so you can check how ancestry is setup anywhere in your code and act based on that. Having orphan_strategy or any other option completely hidden is not a good thing.

[kbrock]

Sure, you can open another PR, bless this PR - what ever works for you.

I don't understand the goal of splitting commits thought.

I make a few hundred PRs a year, and I merge more than that. So in a few years, it is hard to remember why a change is made. Sometimes a specific change is intentional, and other times it is just the first solution that works or even a typo that still passes the tests. So when debugging, it is helpful to include as much context around intent.

For me, keeping move, search replace, and changes in logic in separate commits provides context. Introducing tests in a commit before a change, and then showing what the code changes in the results provides context, too. I'm sure you have rules of your own.

In 10 years, ruby, rails, and use cases have changed quite a lot. Features for this product have as well. So bugs creep in. Every little bit of documentation around intent is helpful in squashing these bugs.

For this PR

Removing the instance variable of ancestry_base_class required a bit of search and replace. So when code changes happened at the same time, it is hard to know if those changes are intended. So I separated out the behavior change.

Example:

- self.base_ancestry_class.none
+ self.class.none
...
- self.ancestry_base_class.siblings_of(self)
+self.class.ancestry_base_class.siblings_of(self)

Is that first one supposed to be self.class.base_ancestry_class.none?

Making this in its own commit feels like over kill, but it is a way to highlight the fact that there is an intentional change here. I mean, this change is very subtle.

Maybe future versions of rails changes none. Maybe future code uses .klass or .unscoped on the returned relation. While the scope's klass did change, a bug probably wouldn't be caught by a test. Hopefully this documentation will help a future developer debug an issue and come up with a better solution. While I'm 99% certain that this is a good change that will not introduce any bugs, I put in a paper trail.

[kbrock]

Unfortunately this lost steam. Will create a pr just around removing the instance variables