Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doc: Add Run migration script instruction to Readme. #1090

Merged
merged 1 commit into from
May 23, 2021

Conversation

mtreacy002
Copy link
Member

Description

Added instruction on how to run migration script to README.md when working on bit branch.

Fixes #1089

Type of Change:

  • Documentation

Code/Quality Assurance Only

  • This change requires a documentation update (software upgrade on readme file)

How Has This Been Tested?

  1. simulated a new contributor who never run application:
    • started on a fresh db by running the application, this created a db with BIT-MS schema. Noticed there was no alembic_version folder.

Screen Shot 2021-05-03 at 8 08 27 am

  • run flask db stamp head to linked the latest alembic version on the repo to the local db

Screen Shot 2021-05-03 at 8 37 36 am

Screen Shot 2021-05-03 at 8 15 16 am

  • continued with register a new user, verify email and login.
  1. simulated an existing contributors of MS backend:
    • prior to running flask db upgrade, contributors would have MS only schema without alembic_version folder in their local_data.db

Screen Shot 2021-05-03 at 7 42 35 am

  • run flask db upgrade to add the latest alembic version on the bit branch to the existing local_data.db.

Screen Shot 2021-05-03 at 8 52 23 am

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

Code/Quality Assurance Only

  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes

@mtreacy002 mtreacy002 marked this pull request as draft May 2, 2021 23:18
modify instructions to new and existing contributors
@codecov
Copy link

codecov bot commented May 2, 2021

Codecov Report

Merging #1090 (cd43681) into bit (f25a0b8) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              bit    #1090   +/-   ##
=======================================
  Coverage   94.50%   94.50%           
=======================================
  Files          39       39           
  Lines        2677     2677           
=======================================
  Hits         2530     2530           
  Misses        147      147           

@mtreacy002 mtreacy002 marked this pull request as ready for review May 2, 2021 23:32
Copy link
Member

@epicadk epicadk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

Copy link
Member

@devkapilbansal devkapilbansal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mtreacy002 I have some questions here:- 👇


This will attach the alembic version number on the repo (if any) to the local db.

* For existing contributors, run the following command:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here existing contributors means whose DB is filled with some entries or those who run flask db upgrade in the past.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those whose db is filled with the previous version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devkapilbansal and @epicadk , I might have to make it clearer here to distinct the 3 situations that might apply to contributors whose dealing with the migration (as I mentioned in this post). Please do share your opinion 😉 .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure @mtreacy002
It would be good to make the documentation as clear as possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, @devkapilbansal , should I add those 3 steps on this PR or on #1038 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that #1038 is required now. Since we are going to merge this branch in develop once completed

@@ -271,8 +290,7 @@ The repository has the following permanent branches:

* **develop** This contains the latest code. All the contributing PRs must be sent to this branch. When we want to release the next version of the app, this branch is merged into the `master` branch. This is the branch that is used in the deployed version of the app on Heroku.

* **bit** This branch is for MS-backend version specific to [BridgeInTech](https://github.com/anitab-org/bridge-in-tech-backend) project. All the contributing PRs related to BIT-MS integration issue must be sent to this branch.<br>
**IMPORTANT!!** If this is your first time setting up the BridgeInTech project, please <b>DO NOT RUN</b> the MS backend server from this branch <b>BEFORE</b> you run the BIT backend server. Failing to do this will mess up the postgres db schemas used in BIT project. More instruction on setting up the BridgeInTech project can be found [here](https://github.com/anitab-org/bridge-in-tech-backend/blob/develop/.github/ENV_SETUP_INSTRUCTION.md).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this point is no more needed? Will not this disturb the Bridge in Tech project configuration?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope because we are integrating so it doesn't make sense to have it any more.

Copy link
Member Author

@mtreacy002 mtreacy002 May 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, the current ms-backend-server from my repo is the one that contributors will continue using until full integration is completed in bit branch, but no need to have this explained in bit branch since they're not using this branch for BIT development atm.

@devkapilbansal devkapilbansal added the Status: Needs Review PR needs an additional review or a maintainer's review. label May 11, 2021
Copy link
Member

@isabelcosta isabelcosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💪🏾

@isabelcosta isabelcosta merged commit e139a9c into anitab-org:bit May 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Review PR needs an additional review or a maintainer's review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants