[NoQA] Do not allow new js files in other folders either#40291
[NoQA] Do not allow new js files in other folders either#40291
Conversation
|
@mananjadhav Please 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] |
|
@fabioh8010 @blazejkustra can you please review? Also wondering if we should specifically check for |
I don't think we need that, NewDot never had |
blazejkustra
left a comment
There was a problem hiding this comment.
Looks good! ✅ Please test it before merging @mountiny (add a js file and check if it fails, then remove it and we should be good)
.github/workflows/typecheck.yml
Outdated
| # - gh pr view is used to list files touched by this PR. Git diff may give false positives if the branch isn't up-to-date with main | ||
| # - wc counts the words in the result of the intersection | ||
| count_new_js=$(comm -1 -2 <(git diff --name-only --diff-filter=A origin/main HEAD -- 'src/*.js') <(gh pr view ${{ github.event.pull_request.number }} --json files | jq -r '.files | map(.path) | .[]') | wc -l) | ||
| count_new_js=$(comm -1 -2 <(git diff --name-only --diff-filter=A origin/main HEAD -- 'src/*.js' '__mocks__/*.js' '.github/*.js' '.storybook/*.js' 'assets/*.js' 'config/*.js' 'desktop/*.js' 'jest/*.js' 'scripts/*.js' 'tests/*.js' 'web/*.js' 'workflow_tests/*.js') <(gh pr view ${{ github.event.pull_request.number }} --json files | jq -r '.files | map(.path) | .[]') | wc -l) |
There was a problem hiding this comment.
'.github/*.js' I'm a bit worried about this one as we generate index.js files that are committed to the repo, if we create a new one Typescript check will fail
There was a problem hiding this comment.
Updated to only look at the libs and scripts subfolders in github and the rest I think we can leave out of this check, its not something updated often
|
@abdulrahuman5196 Please 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] |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
|
✋ 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: 1.4.63-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.4.63-21 🚀
|

Details
We do not want to allow any new JS files to the repo so update the typecheck to fail if new file is added
Fixed Issues
$ #39123
PROPOSAL:
Tests
N/A
Offline tests
N/A
QA Steps
N/A
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop