Add ObjectName class for representing myschema.myobject references

zcmarine commented 6 years ago

This is the worst PR I've ever made. To try to justify this ugly mess so I don't feel so lousy: the choice was between one huge, incomprehensible PR (this) and 48 tiny PRs that were comprehensible and would leave the repo in a good place. My reasons for going this route were:

This PR represents the "atomic" change we're looking for: moving from not using ObjectName instances to using them everywhere. Each of the 48 intermediate commits would all leave the repo in a weird half-and-half state, though admittedly the tests pass at each commit.
48 PRs would have resulted in a LOT of back-and-forth and a ton of time. Given that I leave for several months of vacation in a few days, I was pushing for speed over proper process.
Everything in this PR is changing plumbing. Basically, where we were using strings before use an instance of a class. While on the one hand it may have been a way to get people to know the plumbing of pgbedrock, it's also a way to burn a lot of goodwill and kill people's interest in reviewing PRs. Rather than having people review plumbing, I'd prefer to talk about features and non-trivial changes.
Although it's possible that I may have screwed something up with my changes to the plumbing, we have 96% test coverage and I've tested this against Squarespace's relatively complex use case, so I feel pretty good about the fact that the changes I made didn't break anything. And if they do (which would suck), we have prior docker images and pypi versions available.

With the above given as an attempt to exonerate myself from guilt (which I still feel), I think it'd be useful to discuss this PR in person and see if there's anything else that would be useful to do for due diligence. Actually reviewing the medley of changes seems like a bad use of time.

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 163

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+1.3%) to 97.235%

Totals
Change from base Build 158:	1.3%
Covered Lines:	1266
Relevant Lines:	1302

💛 - Coveralls

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 163

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+1.3%) to 97.235%

Totals
Change from base Build 158:	1.3%
Covered Lines:	1266
Relevant Lines:	1302

Squarespace / pgbedrock