Skip to content

Conversation

@maxceem
Copy link
Contributor

@maxceem maxceem commented Mar 27, 2019

The work initially has been done in the CODE challenge https://www.topcoder.com/challenges/30086308/?type=develop but has various issues and didn't get the passing score.

It was fixed via private task https://www.topcoder.com/challenges/30087019

All the issues were well elaborated. In general the quality of submission looks good.

maxceem added 2 commits March 27, 2019 14:52
…efactoring (submission + final fixes)

also includes small error message fixes and supporting 'null' for scope, phases, form, planConfig, priceConfig for create and update endpoints of projectTemplates
@maxceem maxceem requested a review from vikasrohit March 27, 2019 07:05
Copy link

@vikasrohit vikasrohit left a comment

Choose a reason for hiding this comment

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

I think we need to refactor the productTemplate as well. If not in this long PR, we can take it separately, but we have to do that as soon as we are done with this one.
Also, I have few comments, apart from that it looks good to me.

@vikasrohit
Copy link

@maxceem merging the PR as we are going to take care of renaming of the scope and phases field in a separate task.

@vikasrohit vikasrohit merged commit 4bf9163 into dev Mar 29, 2019
@maxceem
Copy link
Contributor Author

maxceem commented Mar 29, 2019

@vikasrohit if you don't run migration scripts from this PR yet we can just rename these fields in migration scripts, rather than creating new migration scripts which would rename them to config in the DB. Or maybe we can revert this PR until this renaming is done. What do you think?

@vikasrohit
Copy link

Yep, I haven't run the scripts yet. I was waiting for the PR to be deployed first. :) Yes, update the current migration script it self.

@vikasrohit vikasrohit deleted the feature/project-templates-refactoring branch July 29, 2019 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants