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

replace with new tn-models-fp #204

Merged
merged 39 commits into from
Aug 23, 2023
Merged

replace with new tn-models-fp #204

merged 39 commits into from
Aug 23, 2023

Conversation

paribaker
Copy link
Contributor

@paribaker paribaker commented Jul 16, 2023

What this does

Moves vue js to the new tn-models-fp library

How to test

Sign up to app
login to app
Start forgot password flow (note emails are not set up)

@oudeismetis oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-204 July 16, 2023 16:46 Inactive
@oudeismetis oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-204 July 16, 2023 16:50 Inactive
@oudeismetis oudeismetis temporarily deployed to tn-spa-bootstrapper-pr-204 July 16, 2023 18:46 Inactive
@oudeismetis oudeismetis had a problem deploying to tn-spa-bootstrapper-pr-204 July 16, 2023 18:54 Failure
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 July 16, 2023 18:54 Inactive
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 July 16, 2023 19:21 Inactive
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 July 16, 2023 19:53 Inactive
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 July 16, 2023 20:31 Inactive
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 July 16, 2023 20:38 Inactive
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 July 16, 2023 20:43 Inactive
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 July 16, 2023 20:47 Inactive
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 July 16, 2023 20:51 Inactive
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 July 16, 2023 20:55 Inactive
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 August 18, 2023 00:04 Inactive
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 August 18, 2023 00:06 Inactive
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 August 18, 2023 16:26 Inactive
@paribaker
Copy link
Contributor Author

@oudeismetis @lakardion okay this is giving me a headache. To resolve once and for all I've added the /api to the axios instance on all 3 client/mobile side apps and removed api or /api from all services folders!

Will test this out on mobile and then check the react side!

@paribaker paribaker had a problem deploying to tn-spa-bootstrapper-pr-204 August 20, 2023 21:40 Failure
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 August 20, 2023 22:00 Inactive
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 August 20, 2023 22:05 Inactive
@paribaker paribaker had a problem deploying to tn-spa-bootstrapper-pr-204 August 20, 2023 22:15 Failure
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 August 20, 2023 22:27 Inactive
@paribaker paribaker temporarily deployed to tn-spa-bootstrapper-pr-204 August 20, 2023 22:46 Inactive
@@ -45,7 +45,7 @@ jobs:
- name: Pytest, McCabe, and Coverage
run: |
cd my_project
pipenv install --dev --deploy --skip-lock
pipenv install --dev --deploy
Copy link
Member

Choose a reason for hiding this comment

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

why did we make this change?

Copy link
Member

Choose a reason for hiding this comment

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

Interesting....looks like they deprecated this flag.
pypa/pipenv#5805

Sounds like this should be replaced with pipenv run pip install
Not sure if it has the --dev and --deploy flag for regular pip though.

The reason why this was here was to test the app with the latest versions of a library (instead of the pinned versions in the lock file). This would encourage devs to version bump more often because you know it works with the latest.
And if some new thing breaks the build, we can investigate it sooner instead of waiting to hit that error when we are trying to version bump.

Copy link
Member

Choose a reason for hiding this comment

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

yeaaa....pip install couldn't possibly have the --dev flag. Lets merge this as-is and open a github issue to figure out a way to bring this back using pipenv

@oudeismetis
Copy link
Member

@oudeismetis @lakardion okay this is giving me a headache. To resolve once and for all I've added the /api to the axios instance on all 3 client/mobile side apps and removed api or /api from all services folders!

Will test this out on mobile and then check the react side!

I appreciate the efforts to keep things standard, though technically it doesn't matter if Vue and React differ a bit in their approach.
For our teams sake though, I'm sure it would be confusing to bounce between 2 projects doing things slightly differently like this.

@paribaker paribaker merged commit 9e9e04f into main Aug 23, 2023
18 checks passed
@paribaker paribaker deleted the feature/vue-to-models-fp branch August 23, 2023 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants