Skip to content

Conversation

@ckramer10
Copy link
Collaborator

@ckramer10 ckramer10 commented Nov 12, 2025

Description

Screenshot 2025-11-12 at 4 30 13 PM

Checklist

  • New tests and/or benchmarks are included
  • Documentation is changed or added
  • If this change updates the UI, screenshots/videos are added and a design review is requested
  • If this change could impact the load on the MongoDB cluster, please describe the expected and worst case impact
  • I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@ckramer10 ckramer10 added the no release notes Fix or feature not for release notes label Nov 12, 2025
@ckramer10 ckramer10 marked this pull request as ready for review November 13, 2025 16:58
@ckramer10 ckramer10 requested a review from a team as a code owner November 13, 2025 16:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds helpful descriptions to FakerJS methods in the mock data generator to improve user understanding of less obvious method behaviors. The changes transform the faker method mappings from simple string arrays to structured objects containing both the method name and an optional description.

Key Changes:

  • Updated MONGO_TYPE_TO_FAKER_METHODS to use objects with method and optional description fields instead of plain strings
  • Modified component logic to handle the new object structure when rendering faker method options
  • Updated test cases to access the method property from the new object structure

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
constants.ts Transforms faker method mappings from string arrays to objects with method names and descriptions
faker-mapping-selector.tsx Updates component to extract method names from objects and pass descriptions to Option components
faker-mapping-selector.spec.tsx Updates tests to access the method property from the new object structure

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +130 to +132
{ method: 'number.binary', description: 'Binary number string' },
{ method: 'number.octal', description: 'Octal number string' },
{ method: 'number.hex', description: 'Hexadecimal number string' },
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

The descriptions for binary, octal, and hex methods are misleading. These methods return numeric values, not strings. The descriptions should be updated to 'Binary number', 'Octal number', and 'Hexadecimal number' respectively to accurately reflect the return type, or clarified as 'Number in binary format' if they return formatted strings.

Suggested change
{ method: 'number.binary', description: 'Binary number string' },
{ method: 'number.octal', description: 'Octal number string' },
{ method: 'number.hex', description: 'Hexadecimal number string' },
{ method: 'number.binary', description: 'Binary number' },
{ method: 'number.octal', description: 'Octal number' },
{ method: 'number.hex', description: 'Hexadecimal number' },

Copilot uses AI. Check for mistakes.
{ method: 'number.octal', description: 'Octal number string' },
{ method: 'number.hex', description: 'Hexadecimal number string' },
{ method: 'commerce.price', description: 'Monetary price with decimals' },
{ method: 'date.weekday', description: 'Day of the week' },
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

The description 'Day of the week' is ambiguous as the method is listed under 'Number' type but the description doesn't clarify the format. Consider specifying whether this returns a numeric index (0-6) or the name of the day to match the expected Number type.

Suggested change
{ method: 'date.weekday', description: 'Day of the week' },
{ method: 'date.weekday', description: 'Numeric index of day of the week (0=Sunday, 6=Saturday)' },

Copilot uses AI. Check for mistakes.
Code: [
{
method: 'lorem.sentence',
description: 'Short line of code',
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

The description 'Short line of code' is inaccurate for lorem.sentence, which generates Lorem Ipsum text, not actual code. Consider 'Short sentence as code placeholder' or similar to clarify that it's placeholder text being used in a code context.

Suggested change
description: 'Short line of code',
description: 'Short sentence as code placeholder',

Copilot uses AI. Check for mistakes.
@ckramer10 ckramer10 changed the title feature(compass-collection):Add descriptions to FakerJS methods - CLOUDP-358516 feature(compass-collection): Add descriptions to FakerJS methods - CLOUDP-358516 Nov 13, 2025
@ckramer10 ckramer10 changed the title feature(compass-collection): Add descriptions to FakerJS methods - CLOUDP-358516 feat(compass-collection): Add descriptions to FakerJS methods - CLOUDP-358516 Nov 13, 2025
@github-actions github-actions bot added the feat label Nov 13, 2025
@ckramer10 ckramer10 requested a review from kylelai1 November 13, 2025 17:49
@kylelai1
Copy link
Collaborator

Are the faker method descriptions compiled by us or retrieved from fakerjs?

@ckramer10
Copy link
Collaborator Author

Are the faker method descriptions compiled by us or retrieved from fakerjs?

@kylelai1 a collaboration between ChatGPT and us 😄. Put it together and then Jacob and Betsy signed off on it after making some changes

@kylelai1
Copy link
Collaborator

Ah ok, I see copilot left some comments about the descriptions. PR looks good if those have been looked over.

Copy link
Collaborator

@kylelai1 kylelai1 left a comment

Choose a reason for hiding this comment

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

LGTM!!

@ckramer10 ckramer10 merged commit 216b92d into main Nov 14, 2025
58 of 60 checks passed
@ckramer10 ckramer10 deleted the CLOUDP-358516 branch November 14, 2025 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat no release notes Fix or feature not for release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants