Skip to content

Conversation

milindl
Copy link
Contributor

@milindl milindl commented Sep 2, 2025

List of changes for making pipeline more efficient:

  1. remove cache clear - should allow node dependencies to be cached from now on as they're cached automatically
  2. reduction in size of agent whenever possible
  3. switch out Linux: Build, test, lint, docs to use arm64 instead of amd64. This will be the most frequently run block
  4. conditional running: Kafka and SR client blocks are run only when there's an applicable change. There's only one block which runs unconditionally on all PRs and that is "Linux: Build, test, lint, docs"
  5. many blocks are now run on PRs, but not run on the master branch anymore - there's no point in repeating things. We don't merge non-passing PRs anyway.

@Copilot Copilot AI review requested due to automatic review settings September 2, 2025 04:27
@milindl milindl requested review from a team as code owners September 2, 2025 04:27
Copy link

@Copilot Copilot AI left a comment

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 optimizes the Semaphore CI pipeline by making it run less frequently, reducing machine resource usage, and removing cache clearing operations. The changes are aimed at improving efficiency and reducing costs while maintaining test coverage.

  • Adds conditional execution rules to prevent unnecessary pipeline runs on version tags
  • Downgrades machine types from size-2 to size-1 for resource optimization
  • Removes cache clearing step to improve build performance
Comments suppressed due to low confidence (1)

.semaphore/semaphore.yml:1

  • The same conditional logic is repeated multiple times. Consider defining this as a YAML anchor or variable to reduce duplication and make maintenance easier.
# This file is managed by ServiceBot plugin - Semaphore. The content in this file is created using a common

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


blocks:
- name: "Linux amd64 (musl): Build and test"
run:
when: "change_in('['/package.json', '/deps', '/lib', '/src', '/test']') AND tag !~ '^v[0-9]\\.'"
Copy link
Preview

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

The change_in condition uses an array syntax with single quotes inside double quotes, which may cause parsing issues. Consider using proper YAML array syntax: change_in(['/package.json', '/deps', '/lib', '/src', '/test'])

Copilot uses AI. Check for mistakes.

@@ -48,6 +49,8 @@ blocks:
- docker run -v "$(pwd):/v" node:18-alpine /v/.semaphore/build-docker-alpine.sh

- name: "Linux arm64 (musl): Build and test"
run:
when: "change_in('['/package.json', '/deps', '/lib', '/src', '/test']') AND tag !~ '^v[0-9]\\.'"
Copy link
Preview

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

The same conditional logic is repeated multiple times. Consider defining this as a YAML anchor or variable to reduce duplication and make maintenance easier.

Copilot uses AI. Check for mistakes.

- name: "Linux arm64: Build and test"
- name: "Linux amd64: Build and test"
run:
when: "change_in('['/package.json', '/deps', '/lib', '/src', '/test']') AND tag !~ '^v[0-9]\\.'"
Copy link
Preview

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

The same conditional logic is repeated multiple times. Consider defining this as a YAML anchor or variable to reduce duplication and make maintenance easier.

Copilot uses AI. Check for mistakes.

@@ -75,6 +80,8 @@ blocks:
- make test

- name: 'macOS arm64/m1: Build and test'
run:
when: "change_in('['/package.json', '/deps', '/lib', '/src', '/test']') AND tag !~ '^v[0-9]\\.'"
Copy link
Preview

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

The same conditional logic is repeated multiple times. Consider defining this as a YAML anchor or variable to reduce duplication and make maintenance easier.

Copilot uses AI. Check for mistakes.

@@ -146,6 +157,8 @@ blocks:
- make integtest

- name: "Linux amd64: Performance"
run:
when: "change_in('['/package.json', '/deps', '/lib', '/src', '/ci/tests']') AND tag !~ '^v[0-9]\\.'"
Copy link
Preview

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

The same conditional logic is repeated multiple times. Consider defining this as a YAML anchor or variable to reduce duplication and make maintenance easier.

Copilot uses AI. Check for mistakes.

@sonarqube-confluent

This comment has been minimized.

@milindl milindl changed the title [WIP - do not merge] Make semaphore pipeline run less frequently, reduce machine sizes, avoid clearing cache Make semaphore pipeline run less frequently, reduce machine sizes, avoid clearing cache Sep 2, 2025
@sonarqube-confluent

This comment has been minimized.

@@ -48,6 +49,8 @@ blocks:
- docker run -v "$(pwd):/v" node:18-alpine /v/.semaphore/build-docker-alpine.sh

- name: "Linux arm64 (musl): Build and test"
run:
when: "change_in(['/package.json', '/lib', '/src', '/test']) AND tag !~ '^v[0-9]\\.' AND branch != 'master'"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should run tests on master commits as it's possible squash and merge succeeds and there are no conflicts but code isn't rebased before that.

@sonarqube-confluent

This comment has been minimized.

@sonarqube-confluent

This comment has been minimized.

@sonarqube-confluent

This comment has been minimized.

@sonarqube-confluent
Copy link

Passed

Analysis Details

0 Issues

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 0 Code Smells

Coverage and Duplications

  • Coverage No coverage information (70.80% Estimated after merge)
  • Duplications No duplication information (2.00% Estimated after merge)

Project ID: confluent-kafka-javascript

View in SonarQube

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.

2 participants