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

General fixes for issues #64

Merged
merged 27 commits into from
Jan 22, 2022
Merged

General fixes for issues #64

merged 27 commits into from
Jan 22, 2022

Conversation

karansampath
Copy link
Member

No description provided.

@karansampath karansampath linked an issue Dec 4, 2021 that may be closed by this pull request
@karansampath karansampath linked an issue Dec 4, 2021 that may be closed by this pull request
@karansampath karansampath linked an issue Dec 4, 2021 that may be closed by this pull request
@karansampath karansampath marked this pull request as ready for review December 4, 2021 04:53
@karansampath
Copy link
Member Author

Deals with #49, #50 and #51.

Copy link
Member

@markwhiting markwhiting left a comment

Choose a reason for hiding this comment

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

A few minor things but mostly looks good! Let me know when you want me to take another pass.

karansampath and others added 3 commits December 8, 2021 18:28
Co-authored-by: Mark Whiting <[email protected]>
Co-authored-by: Mark Whiting <[email protected]>
@karansampath karansampath linked an issue Dec 12, 2021 that may be closed by this pull request
@karansampath karansampath linked an issue Dec 13, 2021 that may be closed by this pull request
@karansampath karansampath linked an issue Dec 13, 2021 that may be closed by this pull request
@karansampath
Copy link
Member Author

karansampath commented Dec 20, 2021

Have tested #49, #50, and #51 and they are working properly. On #48, I'm not sure why it's not working completely well (radio button doesn't seem to be active). Slightly unsure how to test #53 and #56 but they seem correct (pretty simple implementations).

@markwhiting

@markwhiting
Copy link
Member

@karansampath — are you still working on #48? Or is this ready for another check?

Could you make a short screen capture walking through all the features so we can see them working.

@karansampath
Copy link
Member Author

All issues except #48 are properly working but I'd like a second look at the code on that (has been commented out). I think the rest of the code is ready to go for now and I can finish that on a separate PR. Have attached a loom showing functionality of the rest below:

https://www.loom.com/share/bbb1dfee77234014b22869c968a0992e

@markwhiting

@karansampath karansampath removed a link to an issue Jan 18, 2022
@markwhiting
Copy link
Member

@karansampath just checking — is this ready to go or is there another part we are waiting on?

@karansampath
Copy link
Member Author

karansampath commented Jan 21, 2022

This is ready, I'll fix #48 in another PR today @markwhiting

@markwhiting markwhiting requested review from sumants-dev and removed request for markwhiting January 21, 2022 20:09
@sumants-dev
Copy link
Collaborator

sumants-dev commented Jan 22, 2022

@karansampath
Looks good to me! The test surveys are functional and the data is inserted into the database. Can you confirm that all outdated types in surveys folder have been replaced? For example, numeric -> number.

Squash and merge, if that is true.

@karansampath
Copy link
Member Author

Yep, all outdated types have been replaced!

@karansampath karansampath merged commit 4f05fe6 into main Jan 22, 2022
@karansampath karansampath deleted the generalfix branch January 22, 2022 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants