Skip to content

Conversation

@AliGebily
Copy link
Contributor

Topcoder Project Service - Import and Export Data - Final Fixes according to this document
https://docs.google.com/document/d/100zi5h_KdiY3ASvGCTsSgoMzveGXxuKnJy3zbwU8QQY/edit#heading=h.9js8rvypxdkm

Copy link
Contributor

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

Thanks, @AliGebily. The logic of indexProjectsRange method became much more clear now.

Just a couple of minor things to fix.

  1. Some changes left which could be reverted:

  2. There is some extra space in the export/index.js file:

    image

1- remove space in export/index.js
2- remove ressign lines for res & acc
@AliGebily AliGebily requested a review from maxceem April 13, 2020 17:16
Copy link
Contributor

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

With respect to params reassign, There are old reassigns similar to what I did. It seems that it was also done for skipping lint 'no-param-reassign' rule.
You can search with this sentence: const req = freq;
You will find it in 10 files, and I fixed it in one of them: src/permissions/project.delete.js
If you liked this fix, then I can apply similar fix for other files, otherwise I can revert changes in this file.

@AliGebily thanks for giving it a try. We prefer to keep pull requests with minimal changes so we could better control what changes we did in PR and to avoid any possible side effects of the changes. Also, we often have several features/branches being developed in parallel and changes across multiple files could lead to the merge conflicts later. So that's why we try to avoid unnecessary changes and try to keep each PR to implement only one feature.

Could you please, revert changes you did for src/permissions/project.delete.js. Other than that this pull request looks good.

@AliGebily AliGebily requested a review from maxceem April 14, 2020 12:04
Copy link
Contributor

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

Perfect.

@maxceem maxceem merged commit bfec5ad into topcoder-platform:feature/export-import Apr 14, 2020
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.

2 participants