Skip to content

Update CI/CD to use GitHub Workflow#1902

Open
jcubic wants to merge 9 commits into
mainfrom
workflow
Open

Update CI/CD to use GitHub Workflow#1902
jcubic wants to merge 9 commits into
mainfrom
workflow

Conversation

@jcubic

@jcubic jcubic commented Apr 21, 2024

Copy link
Copy Markdown
Member

Another attempt to merge GitHub workflow to main.

@jcubic jcubic mentioned this pull request Apr 21, 2024
16 tasks
@jcubic

jcubic commented Apr 21, 2024

Copy link
Copy Markdown
Member Author

@seanpoulter can you check why the tests are failing?

@seanpoulter

Copy link
Copy Markdown
Contributor

Could you please try again @isomorphic-git-bot?

@jcubic

jcubic commented Jul 29, 2024

Copy link
Copy Markdown
Member Author

The bot doesn't work anymore, I think it was using prettier to fix the code, but it don't do this anymore.

@jcubic

jcubic commented Jul 29, 2024

Copy link
Copy Markdown
Member Author

If you updated your branch (from a fork) I will need to merge the changes.

@seanpoulter

Copy link
Copy Markdown
Contributor

The bot doesn't work anymore, I think it was using prettier to fix the code, but it don't do this anymore.

Ah, we probably need to grant it permissions to modify the content.

@jcubic

jcubic commented Jul 31, 2024

Copy link
Copy Markdown
Member Author

The bot doesn't work anymore, I think it was using prettier to fix the code, but it don't do this anymore.

Ah, we probably need to grant it permissions to modify the content.

I don't even know where to look at for that bot.

@jcubic jcubic force-pushed the workflow branch 3 times, most recently from df60de6 to 460175b Compare October 28, 2024 07:13
@jcubic jcubic changed the title Workflow Update CI/CD to use GitHub Workflow Oct 30, 2024
@jcubic

jcubic commented Oct 30, 2024

Copy link
Copy Markdown
Member Author

Can we do anything with this issue? It looks like there's a timeout error for running GitHub action.

Elapsed Time: 5:59:576 min/sec/ms
Error: The operation was canceled.

If this is the issue, then maybe we can remove some old browsers. Or add parallelization if it's not already added.

There are also those errors:

Error:  GitHubService HTTP_401 :: Bad credentials
[INFO] Retrieving comparison
Error:  Unable to fetch fileDetails for baseBranch=undefined from https://service.bundlewatch.io/store code=Request failed with status code 401
[INFO] undefined is not a branch to track, no results saved
Error:  GitHubService HTTP_401 :: Bad credentials

@seanpoulter

Copy link
Copy Markdown
Contributor

@jcubic, could you please rerun the workflows so we can see the logs?

@jcubic

jcubic commented Apr 15, 2025

Copy link
Copy Markdown
Member Author

I don't know how, I can only restart isomorphic-git-PR, but that one is Azure.

I can invite you to be part of the org.

@jcubic

jcubic commented Apr 17, 2025

Copy link
Copy Markdown
Member Author

Let me know if you want to be part of the org, I don't remember if you said that you don't want to. There are few people in the org already and they are not active. There are no obligations after you join.

@seanpoulter

Copy link
Copy Markdown
Contributor

Here's the documentation to retry a workflow. I'm happy to jump on a call if you want to walk through it or discuss this synchronously.

I'm happy to join the organization if it'll help grant me permissions to contribute to this branch and run the GitHub Workflows so we can test this and get it resolved sooner. 👍

I was going to suggest working in very small steps to get this moving. First, we've got to get this running with at least one version of Node.

@jcubic

jcubic commented Apr 22, 2025

Copy link
Copy Markdown
Member Author

Invited you as a member. Let me know if you have all access you need.

@jcubic

jcubic commented Apr 22, 2025

Copy link
Copy Markdown
Member Author

It seems it work what was the issue? Let me know when this is ready and can be merged.

@seanpoulter

seanpoulter commented Apr 22, 2025

Copy link
Copy Markdown
Contributor

One of three of checks timed out after 30 minutes the first time. I retried it and it passed. I don't know why it was failing before. I can't see the logs. Before we merge, I'd suggest we:

  • Increase the timeout from 30 to 60 minutes
  • Confirm you want all those changes (like removing tweet-tweet and switching to Node.js v16)
  • Revert the changes to the Azure pipeline so we still publish to npm and the website from there
  • Remove the change to push to npm when we merge to main. We should test on beta first.

After we merge, I'd suggest we:

  • Reduce the runtime by running each browser in parallel. We're running the test script from package-scripts.cjs#L113-L123 which would be to something like:
graph LR
  Start@{ shape: circle, label: "  " } -->  B

   subgraph Build
   B(lint<br>test.typecheck) --> D(build) --> Jest(test.setup<br>test.jest) --> E(Cache)
    end
    
   subgraph "Test (Browser)"
    E --> G(test.setup<br>test.karma - Browser 1)
    E --> H(test.setup<br>test.karma for Browser ...)
    E --> I(test.setup<br>test.karma for Browser n)
    end

    G --> End@{ shape: dbl-circ, label: "  " }
    H --> End
    I --> End
Loading

@jcubic

jcubic commented Apr 22, 2025

Copy link
Copy Markdown
Member Author

Sounds like a plan, go for it. Let me know when you will want something from me.

@seanpoulter seanpoulter requested a review from Copilot April 22, 2025 20:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates the CI/CD configuration to use GitHub Workflow while streamlining several package scripts and removing an obsolete helper file. Key changes include:

  • Enhancements in package-scripts.cjs to add NODE_OPTIONS for Node.js v17+ and updates to webpack, jest, and karma commands.
  • Disabling steps in azure-pipelines.yml by commenting them out.
  • Removal of the tweet.cjs helper file.
  • Introduction of a new GitHub workflow configuration in .github/workflows/ci_cd.yml.

Reviewed Changes

Copilot reviewed 5 out of 8 changed files in this pull request and generated 2 comments.

File Description
package-scripts.cjs Updated Node.js options handling and command modifications for CI/CD.
azure-pipelines.yml Commented out legacy CI steps for npm publishing and website updates.
tests/helpers/tweet.cjs Removed an outdated helper used for tweeting.
.github/workflows/ci_cd.yml Added a new GitHub workflow configuration for CI/CD pipelines.
Files not reviewed (3)
  • .nvmrc: Language not supported
  • .releaserc: Language not supported
  • package.json: Language not supported

Comment thread package-scripts.cjs
Comment thread package-scripts.cjs Outdated
@seanpoulter seanpoulter requested a review from Copilot April 23, 2025 00:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates the CI/CD pipeline by introducing a new GitHub Actions workflow and adjusts the project’s build scripts to support Node.js v17 changes.

  • Added Node.js v17 OpenSSL legacy provider configuration and updated build commands in package-scripts.cjs
  • Removed the tweet helper script, which is no longer used
  • Introduced a new GitHub Actions workflow to drive CI/CD

Reviewed Changes

Copilot reviewed 4 out of 7 changed files in this pull request and generated 1 comment.

File Description
package-scripts.cjs Added OpenSSL legacy provider support and updated commands for webpack, jest, and karma
tests/helpers/tweet.cjs Removed no-longer-needed tweet helper script
.github/workflows/ci_cd.yml Added a new GitHub Actions workflow for CI/CD
Files not reviewed (3)
  • .nvmrc: Language not supported
  • .releaserc: Language not supported
  • package.json: Language not supported
Comments suppressed due to low confidence (2)

package-scripts.cjs:146

  • The removal of the retry3 logic for the jest command changes the test behavior in CI. Please confirm that this change is intentional and that the lack of retry behavior in CI is acceptable.
jest: `jest --ci --coverage`,

.github/workflows/ci_cd.yml:33

  • Using fromJSON with a formatted string for node-version inputs relies on the input being correctly formatted. Validate that the comma-separated input converts to a proper JSON array to avoid runtime errors in the workflow.
node-version: ${{ fromJSON(format('[{0}]', inputs.node-version || '14, 16')) }}

Comment thread package-scripts.cjs Outdated
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.

3 participants