Skip to content

Ci fix - #252

Open
lm-sousa wants to merge 7 commits into
multi-weaverfrom
ci-fix
Open

Ci fix#252
lm-sousa wants to merge 7 commits into
multi-weaverfrom
ci-fix

Conversation

@lm-sousa

@lm-sousa lm-sousa commented Sep 3, 2026

Copy link
Copy Markdown
Member

No description provided.

lm-sousa and others added 7 commits January 20, 2026 19:08
Checkouts correct branch of specs-java-libs
GitHub does not fire the pull_request event when the synthetic merge
ref cannot be created, which silenced CI for conflicted PRs. The
workflow never used the merge ref anyway: branch pushes report the same
head-SHA checks to the PR's Checks tab. Skip runs for branch deletions,
where the event SHA falls back to the default branch and would mislead
the branch resolver.
Copilot AI lite review requested due to automatic review settings September 3, 2026 04:29
@sonarqubecloud

sonarqubecloud Bot commented Sep 3, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 955d602cb3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/nightly.yml

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.

🟡 Changes recommended

The updated nightly workflow likely breaks downstream dependency checkouts because build-java job outputs are no longer reliably populated for needs.build-java.outputs.*.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates Clava’s GitHub Actions workflows to improve CI reliability (branch/dependency ref resolution, reproducible checkouts, and publish gating), and bumps the Clava-JS package version.

Changes:

  • Replaced custom dependency ref resolution logic in workflows with specs-feup/branch-resolver@v1, added full-history checkouts, and adjusted triggers/conditions.
  • Updated the nightly JS build matrix/publish logic and introduced an npm workspace override during CI setup.
  • Fixed the legacy Ant/LARA 2.0 workflow’s dependency checkout parameters; bumped Clava-JS version to 3.5.1.
File summaries
File Description
Clava-JS/package.json Bumps package version (likely to align with CI/publish behavior).
.github/workflows/nightly.yml Refactors nightly CI to use branch resolver, changes JS matrix/publish gating, and adds safeguards around branch deletion events.
.github/workflows/copilot-setup-steps.yml Updates Copilot setup workflow triggers and adds build steps for Weaver + JS workspace build.
.github/workflows/ant-lara-2.0-legacy.yml Aligns legacy build dependency resolution to the lara-2.0-legacy commit for both dependencies.
Review details

Suppressed comments (1)

.github/workflows/nightly.yml:62

  • This workflow now relies on specs-feup/branch-resolver setting env vars, but no step is exporting those values as step outputs for the job outputs to consume. Add an explicit step that writes lara_ref/specs_ref to $GITHUB_OUTPUT (and reference it from build-java.outputs).
      - name: Determine repository refs
        uses: specs-feup/branch-resolver@v1
        with:
          source-directory: clava
          dependencies: |
            lara specs-feup/lara-framework
            specs specs-feup/specs-java-libs
  • Files reviewed: 4/4 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/nightly.yml
Comment thread .github/workflows/copilot-setup-steps.yml
Comment thread .github/workflows/nightly.yml
Comment thread .github/workflows/nightly.yml

Copilot AI commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

One or more custom setup steps configured for this repository failed during this Copilot code review run:

Build Weaver

Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.

Note

You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.

@lm-sousa

lm-sousa commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

Note

glm-5.3-flash responding of behalf of @lm-sousa

Dismissing the "Changes recommended" headline: job outputs are evaluated on the runner at the end of the job, so variables written via GITHUB_ENV mid-job are available when outputs are resolved — outputs: x: ${{ env.x }} is the standard idiom for forwarding GITHUB_ENV values to downstream jobs. This is confirmed in the same workflow: the Echo checks step of build-java reads ${{ env.lara_ref }}/${{ env.specs_ref }} successfully, so needs.build-java.outputs.* will not be empty. Replied and resolved inline.

The remaining comments about the Node test matrix vs. the documented runtimes are real, but they are addressed by the README updates higher in this stack (ts6/vitest). Other Copilot comments (unpinned Node in the Copilot-agent setup workflow, hypothetical Publish JS condition coupling) are nitpicks, left as-is.

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