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

[#9369] Update script sort-usermap to remove duplicates in userMapData.json #9370

Merged
merged 4 commits into from Jan 27, 2019

Conversation

monmanuela
Copy link
Contributor

@monmanuela monmanuela commented Jan 25, 2019

Fixes #9369

PR Checklist

Ensure that you have:

  • Read and understood our PR guideline: https://github.com/TEAMMATES/teammates/blob/master/docs/process.md#step-4-submit-a-pr
    • Added the issue number to the "Fixes" keyword above
    • Titled the PR as specified in the abovementioned document
  • Made your changes on a branch other than master and release
  • Gone through all the changes in this PR and ensured that:
    • They addressed one (and only one) issue
    • No unintended changes were made
  • Run and passed static analysis: ./gradlew lint and npm run lint
  • Added/updated tests, if changes in functionality were involved
  • Added/updated documentation to public APIs (classes, methods, variables), if applicable

Outline of Solution

I updated the sort-usermap script to remove duplicates too using lodash sortedUniq()

@monmanuela monmanuela changed the title [#9369] Script usermap [#9369] Update script sort-usermap to remove duplicates in userMapData.json Jan 25, 2019
sort-usermap.js Outdated
@@ -1,13 +1,15 @@
const fs = require('fs');
const _ = require('lodash');
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 we have lodash dependency and it's an overkill to include that just for this script.

If enforcing uniqueness is your target, you can use ES6 Set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a transitive dependency to lodash, but I can see why this can be an overkill. Will use set then!

@wkurniawan07 wkurniawan07 self-assigned this Jan 26, 2019
@wkurniawan07 wkurniawan07 added the s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging label Jan 26, 2019
@wkurniawan07 wkurniawan07 merged commit 2bd32ac into TEAMMATES:master Jan 27, 2019
@wkurniawan07 wkurniawan07 added this to the V7.0.0-alpha.0 milestone Jan 27, 2019
@monmanuela monmanuela deleted the script-usermap branch January 27, 2019 13:17
@wkurniawan07 wkurniawan07 added the c.Task Other non-user-facing works, e.g. refactoring, adding tests label Jan 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.Task Other non-user-facing works, e.g. refactoring, adding tests s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update script sort-usermap to remove duplicates in userMapData.json
2 participants