Skip to content

Comments

Rename workflow to "Tests" and remove examples directory#189

Closed
Copilot wants to merge 6 commits intomainfrom
copilot/rename-github-actions-job
Closed

Rename workflow to "Tests" and remove examples directory#189
Copilot wants to merge 6 commits intomainfrom
copilot/rename-github-actions-job

Conversation

Copy link
Contributor

Copilot AI commented Nov 20, 2025

Rename GitHub Actions workflow and remove examples directory

  • Rename .github/workflows/test-cn-quickstart.yml to .github/workflows/tests.yml
  • Update workflow name from "Test CN-Quickstart Integration" to "Tests"
  • Update job name from test-cn-quickstart to tests
  • Delete the examples/ directory (no longer needed)
  • Remove example:connect script from package.json
  • Remove examples/**/* from tsconfig.lint.json includes
  • Resolve merge conflicts with base branch changes
  • Update workflows to use recursive submodule checkout
  • Remove redundant submodule initialization steps
  • Merge with v0.0.135 to include latest changes
Original prompt

Rename our test-cn-quickstart.yml GitHub Actions job. That file / job represents the entire workflow now, let's use a more simple and general name like Tests

delete examples/ folder we don't need that dir anymore now that the tests run.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: HardlyDifficult <14855515+HardlyDifficult@users.noreply.github.com>
Copilot AI changed the title [WIP] Rename GitHub Actions job to Tests Rename workflow to "Tests" and remove examples directory Nov 20, 2025
@HardlyDifficult HardlyDifficult marked this pull request as ready for review November 20, 2025 22:13
@HardlyDifficult HardlyDifficult enabled auto-merge (squash) November 20, 2025 22:13
@HardlyDifficult
Copy link
Collaborator

@copilot fix merge conflicts

Co-authored-by: HardlyDifficult <14855515+HardlyDifficult@users.noreply.github.com>
@HardlyDifficult
Copy link
Collaborator

@copilot retry

Copy link
Contributor Author

Copilot AI commented Nov 21, 2025

@copilot retry

Merge conflicts have been successfully resolved in commit 8134a32. The workflow has been renamed to tests.yml with all the latest changes from the base branch (submodules moved to libs/, splice submodule added, etc.).

@HardlyDifficult
Copy link
Collaborator

@copilot .github/workflows/publish.yml:23 should be recursive and
.github/workflows/publish.yml:34
.github/workflows/publish.yml:35 can be removed, right?
.github/workflows/tests.yml:22 should be recursive
.github/workflows/tests.yml:82 removed.

… steps

Co-authored-by: HardlyDifficult <14855515+HardlyDifficult@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 21, 2025

@copilot .github/workflows/publish.yml:23 should be recursive and
.github/workflows/publish.yml:34
.github/workflows/publish.yml:35 can be removed, right?...

Fixed in a2fa20b. Changed both workflows to use submodules: recursive at checkout and removed the redundant submodule initialization steps.

@HardlyDifficult
Copy link
Collaborator

@copilot fix the merge conflicts

auto-merge was automatically disabled November 21, 2025 13:35

Head branch was pushed to by a user without write access

Copy link
Contributor Author

Copilot AI commented Nov 21, 2025

@copilot fix the merge conflicts

Merge conflicts resolved in ca93749. Successfully merged with v0.0.135 while preserving the workflow improvements (recursive submodule checkout).

@HardlyDifficult HardlyDifficult enabled auto-merge (squash) November 21, 2025 14:22
@claude
Copy link

claude bot commented Nov 21, 2025

Claude finished @Copilot's task —— View job


Code Review Summary

I've completed a thorough review of this pull request that renames the GitHub Actions workflow and removes the examples directory.

Positive Changes

  1. Clean workflow rename: Successfully renamed from "Test CN-Quickstart Integration" to "Tests" with simplified job naming
  2. Complete examples cleanup: Properly removed examples directory and all references including:
    • Deleted example:connect script from package.json
    • Removed examples/**/* from tsconfig.lint.json includes
  3. Improved submodule handling: Updated workflows to use submodules: recursive for more robust checkout
  4. Reduced redundancy: Removed redundant submodule initialization steps from workflows

🚨 Issues Found

  1. Duplicate submodule entry in .gitmodules (Line 7-9):

    [submodule "libs/cn-quickstart"]
    	path = libs/cn-quickstart
    	url = https://github.com/digital-asset/cn-quickstart
    

    This duplicates lines 1-3. This could cause issues with git submodule operations.

  2. Node.js version inconsistency:

    • tests.yml uses Node.js 20 (line 27)
    • publish.yml uses Node.js 18 (line 28)

    For consistency and to use the latest LTS, consider standardizing on Node.js 20.

📋 Detailed File Analysis

.github/workflows/tests.yml:

  • ✅ Proper workflow structure with appropriate triggers
  • ✅ Good use of caching for dependencies and build artifacts
  • ✅ Comprehensive logging on failure
  • ⚠️ Consider using Node.js 20 to match latest LTS

.github/workflows/publish.yml:

  • ✅ Proper permissions and security practices
  • ✅ Comprehensive release workflow
  • ⚠️ Consider updating to Node.js 20 for consistency

package.json:

  • ✅ Clean removal of example-related script
  • ✅ No hanging dependencies or references

tsconfig.lint.json:

  • ✅ Proper cleanup of examples path from includes

🔒 Security & Best Practices

  • ✅ No hardcoded secrets or sensitive data
  • ✅ Proper use of GitHub secrets for tokens
  • ✅ Appropriate permissions in workflows
  • ✅ Good use of timeout settings for jobs

🧪 Test Coverage Impact

The removal of examples directory is clean and shouldn't impact existing test coverage since examples were primarily demonstration code, not part of the test suite.

💡 Recommendations

  1. Fix duplicate submodule: Remove duplicate libs/cn-quickstart entry from .gitmodules
  2. Standardize Node.js version: Consider updating publish.yml to use Node.js 20
  3. Consider workflow consolidation: Since both workflows now serve different purposes, the current separation is appropriate

Overall Assessment

This is a well-executed cleanup PR that successfully:

  • Simplifies workflow naming conventions
  • Removes unnecessary example code
  • Improves submodule handling
  • Maintains all functionality

The only critical issue is the duplicate submodule configuration which should be fixed before merging.


auto-merge was automatically disabled December 12, 2025 14:43

Pull request was closed

@HardlyDifficult HardlyDifficult deleted the copilot/rename-github-actions-job branch December 31, 2025 16:25
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