Skip to content

Commit

Permalink
Merge pull request #7 from Expensify/main
Browse files Browse the repository at this point in the history
Even with main
  • Loading branch information
graylewis committed Nov 26, 2023
2 parents 2d578dd + d985a0e commit c6eb49e
Show file tree
Hide file tree
Showing 1,009 changed files with 68,445 additions and 46,481 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
**/node_modules/*
**/dist/*
.github/actions/**/index.js"
docs/vendor/**
3 changes: 0 additions & 3 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -1,5 +1,2 @@
# Every PR gets a review from an internal Expensify engineer
* @Expensify/pullerbear

# Every PR that touches redirects gets reviewed by ring0
docs/redirects.csv @Expensify/infra
22 changes: 22 additions & 0 deletions .github/ISSUE_TEMPLATE/NewLibraryRequest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
name: New Library Request
about: Use this when you want to propose adding a new library to package.json (dev-dependencies excluded)
labels: Weekly, AutoAssignerAppLibraryReview
---
In order to properly evaluate if a new library can be added to `package.json`, please fill out this request form. It will be automatically assigned someone from our review team that will go through and vet the library.

Note: This is only for production dependencies. While we don't want people to add packages to dev-dependencies willy-nilly, we recognize that there isn't as great of a need there to secure them.

# Name of library:

## Details
- Link to package:
- Problem solved by using this package:
- Number of stars in GH:
- Number of monthly downloads:
- Number of releases in the last year:
- Level of activity in the repo:
- Alternatives:
- Are security concerns brought up and addressed in the library's repo?
- How many dependencies does this lib use that will be brought into our code?
- What will the effect be on the bundle size of our code?
12 changes: 0 additions & 12 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,6 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c
- [ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
- [ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
- [ ] I verified that if a function's arguments changed that all usages have also been updated correctly
- [ ] If a new component is created I verified that:
- [ ] A similar component doesn't exist in the codebase
- [ ] All props are defined accurately and each prop has a `/** comment above it */`
- [ ] The file is named correctly
- [ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- [ ] The only data being stored in the state is data necessary for rendering and nothing else
- [ ] If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes
- [ ] For Class Components, any internal methods passed to components event handlers are bound to `this` properly so there are no scoping issues (i.e. for `onClick={this.submit}` the method `this.submit` should be bound to `this` in the constructor)
- [ ] Any internal methods bound to `this` are necessary to be bound (i.e. avoid `this.submit = this.submit.bind(this);` if `this.submit` is never passed to a component event handler like `onClick`)
- [ ] All JSX used for rendering exists in the render method
- [ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
- [ ] If any new file was added I verified that:
- [ ] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
- [ ] If a new CSS style is added I verified that:
Expand All @@ -116,7 +105,6 @@ This is a checklist for PR authors. Please make sure to complete all tasks and c
- [ ] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
- [ ] If a new page is added, I verified it's using the `ScrollView` component to make it scrollable when more elements are added to the page.
- [ ] If the `main` branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the `Test` steps.
- [ ] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

### Screenshots/Videos
<details>
Expand Down
29 changes: 29 additions & 0 deletions .github/actions/composite/buildAndroidAPKDelta/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: Build an Android apk
description: Build an Android apk for an E2E test build and upload it as an artifact

inputs:
ARTIFACT_NAME:
description: The name of the workflow artifact where the APK should be uploaded
required: true

runs:
using: composite
steps:
- uses: Expensify/App/.github/actions/composite/setupNode@main

- uses: ruby/setup-ruby@a05e47355e80e57b9a67566a813648fa67d92011
with:
ruby-version: "2.7"
bundler-cache: true

- uses: gradle/gradle-build-action@3fbe033aaae657f011f88f29be9e65ed26bd29ef

- name: Build APK
run: npm run android-build-e2edelta
shell: bash

- name: Upload APK
uses: actions/upload-artifact@65d862660abb392b8c4a3d1195a2108db131dd05
with:
name: ${{ inputs.ARTIFACT_NAME }}
path: android/app/build/outputs/apk/e2e/release/app-e2edelta-release.apk
6 changes: 3 additions & 3 deletions .github/actions/composite/setupGitForOSBotifyApp/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ runs:
shell: bash
run: |
if [[ -f .github/workflows/OSBotify-private-key.asc.gpg ]]; then
echo "::set-output name=key_exists::true"
echo "key_exists=true" >> "$GITHUB_OUTPUT"
fi
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4
if: steps.key_check.outputs.key_exists != 'true'
with:
sparse-checkout: |
Expand Down
2 changes: 1 addition & 1 deletion .github/actions/composite/setupNode/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ description: Set up Node
runs:
using: composite
steps:
- uses: actions/setup-node@8c91899e586c5b171469028077307d293428b516
- uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'
cache: npm
Expand Down
2 changes: 1 addition & 1 deletion .github/actions/javascript/authorChecklist/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ inputs:
description: Auth token for New Expensify Github
required: true
runs:
using: 'node16'
using: 'node20'
main: './index.js'
67 changes: 0 additions & 67 deletions .github/actions/javascript/authorChecklist/authorChecklist.js

This file was deleted.

163 changes: 163 additions & 0 deletions .github/actions/javascript/authorChecklist/authorChecklist.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
import * as core from '@actions/core';
import * as github from '@actions/github';
import escapeRegExp from 'lodash/escapeRegExp';
import CONST from '../../../libs/CONST';
import GithubUtils from '../../../libs/GithubUtils';
import newComponentCategory from './categories/newComponentCategory';

const pathToAuthorChecklist = `https://raw.githubusercontent.com/${CONST.GITHUB_OWNER}/${CONST.APP_REPO}/main/.github/PULL_REQUEST_TEMPLATE.md`;
const checklistStartsWith = '### PR Author Checklist';
const checklistEndsWith = '\r\n### Screenshots/Videos';

const prNumber = github.context.payload.pull_request?.number;

const CHECKLIST_CATEGORIES = {
NEW_COMPONENT: newComponentCategory,
};

/**
* Look at the contents of the pull request, and determine which checklist categories apply.
*/
async function getChecklistCategoriesForPullRequest(): Promise<Set<string>> {
const checks = new Set<string>();
const changedFiles = await GithubUtils.paginate(GithubUtils.octokit.pulls.listFiles, {
owner: CONST.GITHUB_OWNER,
repo: CONST.APP_REPO,
// eslint-disable-next-line @typescript-eslint/naming-convention
pull_number: prNumber,
// eslint-disable-next-line @typescript-eslint/naming-convention
per_page: 100,
});
const possibleCategories = await Promise.all(
Object.values(CHECKLIST_CATEGORIES).map(async (category) => ({
items: category.items,
doesCategoryApply: await category.detect(changedFiles),
})),
);
for (const category of possibleCategories) {
if (category.doesCategoryApply) {
for (const item of category.items) {
checks.add(item);
}
}
}
return checks;
}

function partitionWithChecklist(body: string): string[] {
const [contentBeforeChecklist, contentAfterStartOfChecklist] = body.split(checklistStartsWith);
const [checklistContent, contentAfterChecklist] = contentAfterStartOfChecklist.split(checklistEndsWith);
return [contentBeforeChecklist, checklistContent, contentAfterChecklist];
}

async function getNumberOfItemsFromAuthorChecklist(): Promise<number> {
const response = await fetch(pathToAuthorChecklist);
const fileContents = await response.text();
const checklist = partitionWithChecklist(fileContents)[1];
const numberOfChecklistItems = (checklist.match(/\[ \]/g) ?? []).length;
return numberOfChecklistItems;
}

function checkPRForCompletedChecklist(expectedNumberOfChecklistItems: number, checklist: string) {
const numberOfFinishedChecklistItems = (checklist.match(/- \[x\]/gi) ?? []).length;
const numberOfUnfinishedChecklistItems = (checklist.match(/- \[ \]/g) ?? []).length;

const minCompletedItems = expectedNumberOfChecklistItems - 2;

console.log(`You completed ${numberOfFinishedChecklistItems} out of ${expectedNumberOfChecklistItems} checklist items with ${numberOfUnfinishedChecklistItems} unfinished items`);

if (numberOfFinishedChecklistItems >= minCompletedItems && numberOfUnfinishedChecklistItems === 0) {
console.log('PR Author checklist is complete 🎉');
return;
}

console.log(`Make sure you are using the most up to date checklist found here: ${pathToAuthorChecklist}`);
core.setFailed("PR Author Checklist is not completely filled out. Please check every box to verify you've thought about the item.");
}

async function generateDynamicChecksAndCheckForCompletion() {
// Generate dynamic checks
console.log('Generating dynamic checks...');
const dynamicChecks = await getChecklistCategoriesForPullRequest();
let isPassing = true;
let didChecklistChange = false;

const body = github.context.payload.pull_request?.body ?? '';

// eslint-disable-next-line prefer-const
let [contentBeforeChecklist, checklist, contentAfterChecklist] = partitionWithChecklist(body);

for (const check of dynamicChecks) {
// Check if it's already in the PR body, capturing the whether or not it's already checked
const regex = new RegExp(`- \\[([ x])] ${escapeRegExp(check)}`);
const match = regex.exec(checklist);
if (!match) {
console.log('Adding check to the checklist:', check);
// Add it to the PR body
isPassing = false;
checklist += `- [ ] ${check}\r\n`;
didChecklistChange = true;
} else {
const isChecked = match[1] === 'x';
if (!isChecked) {
console.log('Found unchecked checklist item:', check);
isPassing = false;
}
}
}

// Check if some dynamic check was added with previous commit, but is not relevant anymore
const allChecks = Object.values(CHECKLIST_CATEGORIES).reduce((acc: string[], category) => acc.concat(category.items), []);

for (const check of allChecks) {
if (!dynamicChecks.has(check)) {
const regex = new RegExp(`- \\[([ x])] ${escapeRegExp(check)}\r\n`);
const match = regex.exec(checklist);
if (match) {
// Remove it from the PR body
console.log('Check has been removed from the checklist:', check);
checklist = checklist.replace(match[0], '');
didChecklistChange = true;
}
}
}

// Put the PR body back together, need to add the markers back in
const newBody = contentBeforeChecklist + checklistStartsWith + checklist + checklistEndsWith + contentAfterChecklist;

// Update the PR body
if (didChecklistChange) {
console.log('Checklist changed, updating PR...');
await GithubUtils.octokit.pulls.update({
owner: CONST.GITHUB_OWNER,
repo: CONST.APP_REPO,
// eslint-disable-next-line @typescript-eslint/naming-convention
pull_number: prNumber,
body: newBody,
});
console.log('Updated PR checklist');
}

if (!isPassing) {
const err = new Error("New checks were added into checklist. Please check every box to verify you've thought about the item.");
console.error(err);
core.setFailed(err);
}

// check for completion
try {
const numberOfItems = await getNumberOfItemsFromAuthorChecklist();
checkPRForCompletedChecklist(numberOfItems, checklist);
} catch (error) {
console.error(error);
if (error instanceof Error) {
core.setFailed(error.message);
}
}
}

if (require.main === module) {
generateDynamicChecksAndCheckForCompletion();
}

export default generateDynamicChecksAndCheckForCompletion;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type Category = {
detect: (changedFiles: Array<{filename: string; status: string}>) => Promise<boolean>;
items: string[];
};

export default Category;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import Category from './Category';
import newComponent from './newComponentCategory';

const categories: Category[] = [newComponent];

export default categories;
Loading

0 comments on commit c6eb49e

Please sign in to comment.