search

hackoregon / backend-examplar-2018

an example dockerized geo-aware django backend
MIT License
4 stars 5 forks source link

Should we switch the default branch to "master" now? #78

Open MikeTheCanuck opened 6 years ago

MikeTheCanuck commented 6 years ago

I have the impression that we're still aligned to a "merge-to-master" pattern/gitflow here, so I'm wondering whether there's any additional value in having the staging branch be our default branch.

Or maybe I'm misunderstanding the intention here - is the idea that by making "staging" the default, accidental commits still don't end up directly affecting the released code, and that we should be merging all PR's to staging and then immediately merging those merges to master?

nam20485 commented 6 years ago

Generally, staging is a whole separate set of infrastructure that mirrors production. So the flow is develop on development, merge to staging to see an automatic deploy of changes to staging, verify changes on staging are good, then simple merge from staging to master to affect the live deployment. Since we don't and aren't planning on having staging infrastructure, I think staging branch is not relevant to our model. With no staging infra, we would be merging from development to staging for really no reason (there is nothing to validate it on) and then merging form staging to master.

Given our model, it might make more sense to develop and test on development branch, and then merge to master to deploy to the live site.

znmeb commented 6 years ago

My recollection was we did that to :"protect" the master branch from kicking off unwanted operations when someone pushed. Staging was meant to be manually merged into master by someone when it was ready for deploying.

MikeTheCanuck commented 6 years ago

PR #86 to address part of this, but we would still need to make the default something other than staging

nam20485 commented 6 years ago

Would we not have the same manual-merge-only protection from development? I envision the workflow being implement in a child branch of development, merge to development, and then if it looks good, do a manual merge to master to deploy.

Really, the only difference is that we call the branch development instead of staging.

nam20485 commented 6 years ago

However, I think it is too dangerous to make the default branch 'master' because it encourages accidentally merging into master, which would cause a deploy, which would be very bad if it was not intended.

MikeTheCanuck commented 6 years ago

My concern is that I don’t have the habit of double-merge, nor the attention span (stretched so thin) to think about whether master is up to date. So if it were left up to me, master could get way out of date.

If one of you are keeping a close eye on this and will continue to monitor after Demo Day, this sounds like a solid plan and I support it. Please don’t rely on me for this discipline.

nam20485 commented 6 years ago

The necessity to have to double merge is the actual protection from accidental deploy.

It's too dangerous to make Master the default branch, because although it's more convenient, that convenience will end up coming back to bite us when someone accidentally develops and commits to Master.

Development should be done on a different branch than master and then merged to Master when an actual conscious decision is made to deploy those changes.

I also would advocate adding protection for the Master branch so that no one can merge to it without two of our approvals.

This all being said, I am not sure that we are concerned about accidentally deploying this repo's code, since it is not deployed to anything facing the public.

All my points are more valid for the actual respective backend repos, bc their Travis build is the one that deploys to production, and which we have to worry about if a deploy happened that we didn't authorize.

On Wed, May 23, 2018, 12:45 PM Mike Lonergan notifications@github.com wrote:

My concern is that I don’t have the habit of double-merge, nor the attention span (stretched so thin) to think about whether master is up to date. So if it were left up to me, master could get way out of date.

If one of you are keeping a close eye on this and will continue to monitor after Demo Day, this sounds like a solid plan and I support it. Please don’t rely on me for this discipline.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/hackoregon/backend-examplar-2018/issues/78#issuecomment-391473184, or mute the thread https://github.com/notifications/unsubscribe-auth/ABvqFPlJyw3bvNO1EjJJD6I0Dv5ao7v9ks5t1bxlgaJpZM4T0FhF .

znmeb commented 6 years ago

We should probably have some kind of "code cutoff" for this repo. Transportation Systems is now working on our own API using it as of 2018-05-20 for a template.

nam20485 commented 6 years ago

Agreed. But I think we need to get a couple PRs in before it's ready for roll out.

For instance: 1.) the unit testing, and 2.) the outstanding PR I have with the deploy tag + docker deploy updates. Also, 3.) The renamed .env variables.

On Wed, May 23, 2018, 3:01 PM M. Edward (Ed) Borasky < notifications@github.com> wrote:

We should probably have some kind of "code cutoff" for this repo. Transportation Systems is now working on our own API using it as of 2018-05-20 for a template.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/hackoregon/backend-examplar-2018/issues/78#issuecomment-391510624, or mute the thread https://github.com/notifications/unsubscribe-auth/ABvqFHKevc0PHbVWieSuDrZbj6zFJ3CVks5t1dxLgaJpZM4T0FhF .

znmeb commented 6 years ago

Yeah - I used the new environment variables and the PostGIS database patch when I did the fork. Those need to be merged.