Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

rename subscriptions to payment_instructions #3652

Merged
merged 8 commits into from
Jul 30, 2015
Merged

Conversation

chadwhitacre
Copy link
Contributor

@chadwhitacre
Copy link
Contributor Author

Ping @rorepo @rohitpaulk @techtonik @kzisme et al.

@chadwhitacre
Copy link
Contributor Author

Ping @techtonik @tshepang @colindean @mattbk for conceptual review.

@tshepang
Copy link
Contributor

see this Comment gratipay/inside.gratipay.com#117 (comment)

@chadwhitacre
Copy link
Contributor Author

[A]dding _instructions does not help anyone understand these things better, and makes for overlong names.

instructions is the base, and payment_/payroll_ is the modifier, rather than the other way around. If we wanted to shorten the table name we could combine them into a single instructions table, with an ENUM('payment', 'payroll') column. Payroll instructions will include tax withholding instructions, which could be NULL where instruction_type is payment. That'd be similar to how we do exchange_routes, where we have a fee_cap that only paypal uses.

@chadwhitacre
Copy link
Contributor Author

I'm starting to code up that approach, and it feels to me like it brings payments and payroll too close together. We don't want to accidentally conflate them, that'd be bad bugs. I think we should keep them as separate tables.

@chadwhitacre
Copy link
Contributor Author

I'm gonna self-merge this pretty soon ...

@chadwhitacre chadwhitacre added this to the Pivot milestone Jul 30, 2015
@mattbk
Copy link
Contributor

mattbk commented Jul 30, 2015

I trust you know what you're doing.

On July 30, 2015 8:25:36 AM CDT, Chad Whitacre notifications@github.com wrote:

I'm gonna self-merge this pretty soon ...


Reply to this email directly or view it on GitHub:
#3652 (comment)

Sent from a phone that, although intelligent, is not street smart.

@chadwhitacre
Copy link
Contributor Author

I much preferred when we had people around to code review. I think we'll get back there again, but I don't think we can wait until then to make progress. :-(

chadwhitacre added a commit that referenced this pull request Jul 30, 2015
rename `subscriptions` to `payment_instructions`
@chadwhitacre chadwhitacre merged commit 687c474 into master Jul 30, 2015
@chadwhitacre chadwhitacre deleted the payment_instructions branch July 30, 2015 15:52
@chadwhitacre
Copy link
Contributor Author

/me takes a backup before deploying ...

@chadwhitacre
Copy link
Contributor Author

/me remembers that it's payday ... :)

@chadwhitacre
Copy link
Contributor Author

#3656 :)

@chadwhitacre
Copy link
Contributor Author

Deployed!

@chadwhitacre chadwhitacre mentioned this pull request Feb 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants