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

Use GithubUtils to access octokit #10415

Merged
merged 2 commits into from
Aug 18, 2022

Conversation

francoisl
Copy link
Contributor

@francoisl francoisl commented Aug 17, 2022

cc'ing @roryabraham for context/familiarity

Details

Attempt to solve https://github.com/Expensify/App/runs/7866938557?check_suite_focus=true:

Fetching tag list from github...
/home/runner/work/_actions/Expensify/App/main/.github/actions/javascript/getDeployPullRequestList/index.js:45
    return octokit.repos.listTags({
                         ^

TypeError: Cannot read property 'listTags' of undefined
    at getTagsOrReleases (/home/runner/work/_actions/Expensify/App/main/.github/actions/javascript/getDeployPullRequestList/index.js:45:26)
    at Object.5847 (/home/runner/work/_actions/Expensify/App/main/.github/actions/javascript/getDeployPullRequestList/index.js:52:1)
    at __nccwpck_require__ (/home/runner/work/_actions/Expensify/App/main/.github/actions/javascript/getDeployPullRequestList/index.js:10798:43)
    at /home/runner/work/_actions/Expensify/App/main/.github/actions/javascript/getDeployPullRequestList/index.js:10815:18
    at Object.<anonymous> (/home/runner/work/_actions/Expensify/App/main/.github/actions/javascript/getDeployPullRequestList/index.js:10816:[12](https://github.com/Expensify/App/runs/7866938557?check_suite_focus=true#step:4:13))
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:[14](https://github.com/Expensify/App/runs/7866938557?check_suite_focus=true#step:4:15))
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)

Tests

Is this testable locally in advance?

QA

  1. Go to https://github.com/Expensify/App/actions/workflows/platformDeploy.yml
  2. Open the first (most recent) "Merge pull request XXX from YYY"
  3. Make sure the step "Post a GitHub comment when platforms are done building and deploying" completes successfully

image

@francoisl francoisl requested a review from roryabraham August 17, 2022 00:31
@francoisl francoisl self-assigned this Aug 17, 2022
@francoisl francoisl requested a review from a team as a code owner August 17, 2022 00:31
@melvin-bot melvin-bot bot requested review from arosiclair and removed request for a team August 17, 2022 00:32
@@ -18,13 +18,13 @@ const itemToFetch = isProductionDeploy ? 'release' : 'tag';
*/
function getTagsOrReleases(fetchReleases) {
if (fetchReleases) {
return octokit.repos.listReleases({
return GithubUtils.octokit.repos.listReleases({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think an alternative would be to do octokit.rest.repos.listReleases() (ref), but this was the only place we don't use GithubUtils, so I went with that for consistency.

@roryabraham roryabraham merged commit a72f85a into main Aug 18, 2022
@roryabraham roryabraham deleted the francois-fixGetDeployPullRequestList branch August 18, 2022 17:51
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @roryabraham in version: 1.1.89-0 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@mvtglobally
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants