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.

Sign up for GitHub

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

[code-infra] CircleCI maintenance #1321

Merged
merged 7 commits into from
Jan 31, 2025
Merged

[code-infra] CircleCI maintenance #1321

Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0569345
Remove React 17 pipeline and fix TS tests
michaldudak Jan 10, 2025
2c27a4a
Unify names of CCI jobs
michaldudak Jan 10, 2025
e0f705a
fix
michaldudak Jan 10, 2025
e144654
Remove unused parameters
michaldudak Jan 10, 2025
ac5e6e4
Simplify updating TS
michaldudak Jan 23, 2025
145a6dd
Merge remote-tracking branch 'upstream/master' into circleci-maintenance
michaldudak Jan 29, 2025
7a76d5a
Merge branch 'master' into circleci-maintenance
michaldudak Jan 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 30 additions & 60 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
version: 2.1
orbs:
aws-cli: circleci/[email protected]
aws-s3: circleci/[email protected]

parameters:
browserstack-force:
Expand All @@ -16,31 +13,17 @@ parameters:
description: The name of the workflow to run
type: string
default: pipeline
e2e-base-url:
description: The base url for running end-to-end test
type: string
default: ''

default-job: &default-job
parameters:
react-version:
description: The version of react to be used
type: string
default: << pipeline.parameters.react-version >>
test-gate:
description: A particular type of tests that should be run
type: string
default: undefined
e2e-base-url:
description: The base url for running end-to-end test
type: string
default: << pipeline.parameters.e2e-base-url >>
environment:
# expose it globally otherwise we have to thread it from each job to the install command
BROWSERSTACK_FORCE: << pipeline.parameters.browserstack-force >>
REACT_VERSION: << parameters.react-version >>
TEST_GATE: << parameters.test-gate >>
AWS_REGION_ARTIFACTS: eu-central-1
COREPACK_ENABLE_DOWNLOAD_PROMPT: '0'
working_directory: /tmp/base-ui
docker:
Expand Down Expand Up @@ -172,7 +155,10 @@ jobs:
- install_js:
react-version: << parameters.react-version >>
- run:
name: Eslint
name: 'Prettier'
command: pnpm prettier --check
- run:
name: ESlint
command: pnpm eslint:ci
- run:
name: Stylelint
Expand All @@ -189,9 +175,6 @@ jobs:
- checkout
- install_js:
react-version: << parameters.react-version >>
- run:
name: '`pnpm prettier` changes committed?'
command: pnpm prettier --check
- run:
name: Generate PropTypes
command: pnpm proptypes
Expand All @@ -214,6 +197,11 @@ jobs:
command: |
pnpm docs:link-check
git add -A && git diff --exit-code --staged
- run:
name: '`pnpm inline-scripts changes committed?'
command: |
pnpm inline-scripts
git add -A && git diff --exit-code --staged
test_types:
<<: *default-job
resource_class: 'medium+'
Expand Down Expand Up @@ -245,18 +233,13 @@ jobs:
- run:
name: Resolve typescript version
command: |
pnpm add typescript@next -d -w
pnpm install typescript@next -d -w
Copy link
Member

Choose a reason for hiding this comment

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

I think the best command I know for this is

pnpm update -r typescript@next

it should update every workspace in the repo where typescript is installed, including the root.

pnpm install typescript@next -d --filter @base-ui-components/react
# log a patch for maintainers who want to check out this change
git --no-pager diff HEAD
- run:
name: Tests TypeScript definitions
command: |
# ignore build failures
# it's expected that typescript@next fails since the lines of the errors
# change frequently. This build is monitored regardless of its status
set +e
pnpm typescript
exit 0
command: pnpm typescript
- restore_cache:
name: Restore generated declaration files
keys:
Expand Down Expand Up @@ -350,71 +333,58 @@ jobs:
command: pnpm -r test:package

workflows:
version: 2
pipeline:
when:
equal: [pipeline, << pipeline.parameters.workflow >>]
jobs:
- checkout:
<<: *default-context
name: 'Checkout'
- test_unit:
<<: *default-context
name: 'JSDOM tests'
requires:
- checkout
- Checkout
Copy link
Member

Choose a reason for hiding this comment

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

These should probably remain checkout as it relies on key, not the name. 🤔
The jobs seem stuck now. :/

Suggested change
- Checkout
- checkout

Copy link
Member

@oliviertassinari oliviertassinari Jan 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

+1 to keep snake case to match with the rest of the codebase and with https://circleci.com/docs/introduction-to-yaml-configurations/ style.

Copy link
Member Author

Choose a reason for hiding this comment

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

It does rely on a name. The job seemed stuck as Github checks expect a job with the old name. I will update the GH configuration once the PR is merged so that other PRs aren't stuck.

Copy link
Member Author

Choose a reason for hiding this comment

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

@oliviertassinari I haven't found anything in the CircleCI that forces or recommends any particular format. In fact, they use different conventions in different parts of the docs (https://circleci.com/docs/configuration-reference/#requires has kebab-case)

I prioritized the readability of GH status checks (especially for external contributors).
Compare
image
with
image

Copy link
Member

Choose a reason for hiding this comment

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

I'm all for more readability if it's just a GH config. 👍

- test_lint:
<<: *default-context
name: 'Linting'
requires:
- checkout
- Checkout
- test_static:
<<: *default-context
name: 'Generated files verification'
requires:
- checkout
- Checkout
- test_types:
<<: *default-context
name: 'Typechecking'
requires:
- checkout
- Checkout
- test_browser:
<<: *default-context
name: 'Browser tests'
requires:
- checkout
- Checkout
- test_regressions:
<<: *default-context
name: 'Regression tests'
requires:
- checkout
- Checkout
- test_e2e:
<<: *default-context
name: 'E2E tests'
requires:
- checkout
- test_bundle_size_monitor:
<<: *default-context
requires:
- checkout
- Checkout
# - test_bundle_size_monitor:
# <<: *default-context
# name: 'Bundle size monitor'
# requires:
# - checkout
- test_package:
<<: *default-context
name: 'Package verification'
requires:
- checkout
react-17:
triggers:
- schedule:
cron: '0 0 * * *'
filters:
branches:
only:
- master
jobs:
- test_unit:
<<: *default-context
react-version: ^17.0.0
- test_browser:
<<: *default-context
react-version: ^17.0.0
- test_regressions:
<<: *default-context
react-version: ^17.0.0
- Checkout
react-18:
when:
equal: [pipeline, << pipeline.parameters.workflow >>]
Expand Down
Loading