-
Notifications
You must be signed in to change notification settings - Fork 308
Conversation
Okay @JessaWitzel, this one's for you (ref)! :-) Your first PR was for inside.gratipay.com ... this one is gonna be your first PR for gratipay.com itself! 💃 Here's what we are looking for here:
Does that give you enough to get started? For broader context, check the project linked in the sidebar here on GitHub (and check the "Details" on the project page). As always, if you're stuck after an hour, ask us a question here or on slack! Okay! Good luck! 👍 💃 🌻 |
Have removed the to-do URL field from the form and the field from the teams table (I think) but when I try to create a team locally I receive gratipay.com/error.spt "Please fill out the 'To-do URL' field." Went into testing and removed the todo_url test but am still receiving error. |
This is how I like to solve problems like this: https://github.com/gratipay/gratipay.com/search?utf8=%E2%9C%93&q=Please+fill+out+the+%27To-do+URL%27+field. (You can ignore i18n files, they are generated for translations.) |
The tests worked for a minute but then I had to clean and re: make schema make fake and the fake data was wrong. Now I am receiving a subprocess error when I try to run tests that stalls on the first test related to honcho? |
Why does make test have so many more failures than make pytest? This passes when I make pytest. |
I ended up removing all references to todo_url in the code, not just those related to the form because it didn't pass tests until I did. I hope that is what you wanted. :) |
!m @JessaWitzel I see the tasks are ticked in #4214 (comment). Does that mean you're done here? If so, go ahead and move it to the "Landing" column on the project (see sidebar) and then ping a few of us here by name to let us know it's ready for review! :) |
Ready for review friends! @whit537 @mattbk @clone1018 |
You know what? I fixed that and then after all of the testing errors I had to redo some things and never checked to see if I could submit the form. I will fix. |
Success after |
@@ -546,7 +546,6 @@ BEGIN; | |||
ALTER TABLE teams ALTER COLUMN getting_paid DROP NOT NULL; | |||
|
|||
ALTER TABLE teams ADD COLUMN onboarding_url text NOT NULL DEFAULT ''; | |||
ALTER TABLE teams ADD COLUMN todo_url text NOT NULL DEFAULT ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Soooo ... schema.sql
is append-only. We shouldn't delete from it like this. Check in the README for how to modify the database. As usual, ping me/us if you get stuck. :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies. There is so much to remember!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No apologies necessary! You're doing great! :-)
, onboarding_url='http://INSIDE.GRATipay.com/' | ||
, todo_url='hTTPS://github.com/GRATIPAY' | ||
)) | ||
, onboarding_url='http://INSIDE.GRATipay.com/')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to be picky but can we match the parentheses formatting here as it was before? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 6062804. :-)
Looks good so far, @JessaWitzel! Good work! 💃 I was able to submit the form locally. 👍 I've left a couple todos (in the ticket description), one tiny and one more involved. |
--https://github.com/gratipay/gratipay.com/pull/4214 | ||
BEGIN; | ||
ALTER TABLE teams DROP COLUMN todo_url; | ||
END; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JessaWitzel Close! Rather than modifying sql/schema.sql
directly, what we want is a file at sql/branch.sql
(it shouldn't exist on your branch; you have to create it and add it to Git). If there's a file at sql/branch.sql
then our deploy script takes care of appending it to sql/schema.sql
after running sql/branch.sql
against the production database. Make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Thank you!
2fcc8a3
to
88226c1
Compare
8db0601
to
0c1252c
Compare
|
Aaaaaaaand #4224. 😩 |
Returning from #4224 ... |
Builds restarted. Will it be as simple as that? |
c2644b5
to
cb71c2a
Compare
Green! Let's see if we can land this ... |
Rebased, was 2596672. |
2596672
to
6b5e2cc
Compare
1619f84
to
cf13cc6
Compare
6b5e2cc
to
8da9b4d
Compare
Hit #4224. Cache cleared for this PR and build restarted ... |
I removed the caches for |
Looks like it's working! 💃 |
I accidentally rebased 2596672 away! Brought it back with:
|
Woo-hoo! Good work, @JessaWitzel! 💃 🌻 |
Closes #4166; part of #4130.
Todo