-
Notifications
You must be signed in to change notification settings - Fork 3.1k
[NoQA] Add CSpell
for spelling checks [Part 1]
#60385
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
Conversation
|
Concierge reviewer checklist:
For more detailed instructions on completing this checklist, see How do I review a HelpDot PR as a Concierge Team member? |
@Valforte @CortneyOfstad One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one observation, and need to check ESLint exception
...y-classic/getting-started/playbooks/Expensify-Playbook-For-US-Based-Bootstrapped-Startups.md
Outdated
Show resolved
Hide resolved
CSpell
for spelling checks [Part 1]CSpell
for spelling checks [Part 1]
Hey, I noticed you changed some webpack configuration files. This can break production builds. Did you remember to run a production build locally to verify they still work? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think we have to do the eslint check here as we already touched a lot of files
@@ -60,7 +60,7 @@ This dictates when reimbursable expenses will export, according to your preferre | |||
|
|||
## Export Settings for Reimbursable Expenses | |||
|
|||
**Expense Reports:** Expensify transactions will export reimbursable expenses as expense reports by default, which will be posted to the payables account designated in NetSuite. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CortneyOfstad Can you confirm please, this sounds right to me, but would be great to get a double check. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CortneyOfstad Can you help to confirm?
@@ -496,7 +496,7 @@ If you have confirmed that your categories are set as Expense Categories in NetS | |||
## What's the difference between a Custom Segment, Custom Record, and Custom List? | |||
|
|||
- **Custom Record**: This is an entirely customizable record type used to store detailed, structured information not covered by standard NetSuite records (e.g., customers, vendors, and transactions). Unlike Segments, Custom Records are designed to manage standalone data, like tracking projects, assets, or other entities. | |||
- **Custom Segment**: A custom classification field similar to standard NetSuite segments like Department, Class, or Location. Typically used when you need to categorize transactions, records, or GL entries for reporting purposes or to separate financials. (NetSuite automatically creates a Custom Record behind the scenes to power the segment.) | |||
- **Custom Segment**: A custom classification field similar to standard NetSuite segments like Department, Class, or Location. Typically used when you need to categorize transactions, records, or GL entries for reporting purposes or to separate financial. (NetSuite automatically creates a Custom Record behind the scenes to power the segment.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like financials is right here cc @CortneyOfstad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the conclusion? @CortneyOfstad
@@ -1,11 +1,11 @@ | |||
--- | |||
title: add-an-expense.md | |||
description: Learn how to add, edit, and manage expenses in Expensify, including SmartScans, per diems, mileage, and group expenses. | |||
description: Learn how to add, edit, and manage expenses in Expensify, including SmartScans, per diem, mileage, and group expenses. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plural is good here too
docs/articles/expensify-classic/workspaces/Configure-Reimbursement-Settings.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Vit Horacek <[email protected]>
@dominictb couple of requested changes, can you please address them? @ishpaul777 can you also review please |
Reviewer Checklist
Screenshots/VideosAndroid: NativeUploading Screen Recording 2025-04-22 at 3.14.15 PM.mov… Android: mWeb ChromeScreen.Recording.2025-04-21.at.2.06.27.AM.moviOS: NativeScreen.Recording.2025-04-21.at.2.32.18.AM.moviOS: mWeb SafariScreen.Recording.2025-04-21.at.2.02.56.AM.movMacOS: Chrome / SafariScreen.Recording.2025-04-21.at.2.01.46.AM.movMacOS: DesktopScreen.Recording.2025-04-22.at.3.28.30.PM.mov |
It LGTM! just resolve conflicts please @dominic |
@ishpaul777 @mountiny All checked! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good!
Co-authored-by: Vit Horacek <[email protected]>
As this is a large PR updating many files, we are not going to update the optional ESLint change here - not emergency |
As there is high potential of a conflict, I am going to merge now |
@mountiny looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
Explained above |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.32-0 🚀
|
@mountiny @ishpaul777
Explanation of Change
Fix spells in all folders and top-level files except
src/
andtests/
folders. Github Action for CSpell has not been integrated here.Fixed Issues
$ #54810
PROPOSAL:
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop