helpwithcovid / covid-volunteers

Organizing and matching volunteers with COVID-19 projects
https://helpwithcovid.com
MIT License
96 stars 76 forks source link

Create business entity #282

Closed shen-sat closed 4 years ago

shen-sat commented 4 years ago

This PR introduces the Business model and its associated new, create and index actions.

Please note:

jamiew commented 4 years ago

This looks good to me overall, although I am still curious about extending and re-using Project for this.

Seems like there is some redundancy in how they both can be user-submitted, how they're approved, can they both be highlighted?, etc. As well as not confusing folks who might be forking this codebase

Fine to merge since that could be addressed and refactored later

shen-sat commented 4 years ago

This looks good to me overall, although I am still curious about extending and re-using Project for this.

Seems like there is some redundancy in how they both can be user-submitted, how they're approved, can they both be highlighted?, etc. As well as not confusing folks who might be forking this codebase

Fine to merge since that could be addressed and refactored later

As I was creating the Business object, I realised the same thing - lots of duplication :(

Would you mind merging this PR and deploying @jamiew? For the next PR, I'll reuse Project and see how that goes

jamiew commented 4 years ago

Are you not able to merge and deploy? I won't be able to do that until this evening at earliest

On Wed, Aug 5, 2020 at 2:10 PM Shen notifications@github.com wrote:

This looks good to me overall, although I am still curious about extending and re-using Project for this.

Seems like there is some redundancy in how they both can be user-submitted, how they're approved, can they both be highlighted?, etc. As well as not confusing folks who might be forking this codebase

Fine to merge since that could be addressed and refactored later

As I was creating the Business object, I realised the same thing - lots of duplication :(

Would you mind merging this PR and deploying @jamiew https://github.com/jamiew? For the next PR, I'll reuse Project and see how that goes

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/helpwithcovid/covid-volunteers/pull/282#issuecomment-669350161, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAO37O6AWQW47JRWPT4D3R7GOC7ANCNFSM4PVW6N6Q .

-- @jamiew https://twitter.com/jamiew | https://jamiedubs.com https://www.jamiedubs.com/

shen-sat commented 4 years ago

It's saying Only those with write access to this repository can merge pull requests. To deploy I can just run the deploy-hwbe.sh file from my console, right? Once the build has passed on circleci

jamiew commented 4 years ago

@shen-sat just added you to the @helpwithcovid/hwc-core team so you shoul have access now. And yes that script will deploy helpwithblackequity branch to the HWBE site

jamiew commented 4 years ago

Oh please note that script does not run db migrations! Would need to do deploy it and then do heroku run 'rails db:migrate' (I think)

shen-sat commented 4 years ago

Cheers @jamiew - I'll try merging and deploying now