tansengming / stripe-rails

A Rails Engine for integrating with Stripe
MIT License
755 stars 123 forks source link

Plan tiers #180

Closed cpsoinos closed 4 years ago

cpsoinos commented 4 years ago

Continued from https://github.com/tansengming/stripe-rails/pull/160

Adds the ability to specify pricingtiers on plans.

fixes #158

jonsgreen commented 4 years ago

@tansengming Just curious if you have a sense of when you might be able to look at this?

tansengming commented 4 years ago

@jonsgreen hopefully I’ll be able to have a look at this over the weekend.

This looks like a big chunk of work, thanks @cpsoinos !

cpsoinos commented 4 years ago

@tansengming thanks for the review! I've addressed your comments, but it looks like the CI checks are stuck. Any idea what's up?

tansengming commented 4 years ago

@cpsoinos it sounds like you need to enable Github Actions on your repo for it to work https://github.community/t5/GitHub-Actions/Run-a-GitHub-action-on-pull-request-for-PR-opened-from-a-forked/td-p/15426

It doesn't look like the actions are running this repo at all https://github.com/tansengming/stripe-rails/actions

cpsoinos commented 4 years ago

@tansengming actions are enabled in my fork of the repo 🤔. In that actions build, tests passed but the check failed due to missing secrets (expected):

.........................

Finished in 2.535377s, 35.1033 runs/s, 53.2465 assertions/s.
89 runs, 135 assertions, 0 failures, 0 errors, 0 skips
Coverage report generated for Unit Tests to /home/runner/work/stripe-rails/stripe-rails/coverage. 465 / 474 LOC (98.1%) covered.
Error: you must supply a CC_TEST_REPORTER_ID ENV variable or pass it via the -r flag

https://github.com/streetsmartslabs/stripe-rails/runs/547173833?check_suite_focus=true

tansengming commented 4 years ago

@cpsoinos One last try: I've updated the github workflow to trigger on pull_request too Hopefully that'll run the checks this time. Just merge from master and see if it works.

I'll merge it anyway if that doesn't work. Thanks for being patient with this.

cpsoinos commented 4 years ago

@cpsoinos One last try: I've updated the github workflow to trigger on pull_request too Hopefully that'll run the checks this time. Just merge from master and see if it works.

I'll merge it anyway if that doesn't work. Thanks for being patient with this.

Almost! It ran the actions this time, but looks like it's still getting caught on the CC_TEST_REPORTER_ID:

........

Finished in 7.489373s, 11.8835 runs/s, 18.0255 assertions/s.
89 runs, 135 assertions, 0 failures, 0 errors, 0 skips
Coverage report generated for Unit Tests to /home/runner/work/stripe-rails/stripe-rails/coverage. 465 / 474 LOC (98.1%) covered.
Error: you must supply a CC_TEST_REPORTER_ID ENV variable or pass it via the -r flag

Tests are passing, however 👍

tansengming commented 4 years ago

yup looks good I'll fix that CC_TEST_REPORTER_ID thing later. Thanks!

cpsoinos commented 4 years ago

Great, thanks @tansengming!

jonsgreen commented 4 years ago

@cpsoinos, @tansengming Thank you both for your hard work on this feature.