Skip to content

feat(migrate): improve GitHub Actions README Generator migration behavior#110

Merged
neilime merged 5 commits intomainfrom
copilot/fix-109
Sep 19, 2025
Merged

feat(migrate): improve GitHub Actions README Generator migration behavior#110
neilime merged 5 commits intomainfrom
copilot/fix-109

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Sep 19, 2025

This PR addresses four critical issues with the GitHub Actions README Generator migration adapter that were causing incorrect migration behavior:

Issues Fixed

1. Incorrect Section Mapping

The branding section was incorrectly mapped to badges instead of header. Both branding and title sections should map to the same header section according to the tool's intended behavior.

Before:

'branding': SectionIdentifier.Badges,  // ❌ Wrong mapping
'title': SectionIdentifier.Header,

After:

'branding': SectionIdentifier.Header,  // ✅ Correct mapping
'title': SectionIdentifier.Header,

2. Unsupported Markers Not Removed

Non-supported markers (like contents) were being preserved in the migrated output instead of being removed while keeping their content. This cluttered the migrated files with invalid markers.

Before:

<!-- start contents -->
Some content here
<!-- end contents -->

After:

Some content here

3. Missing Required Sections

The migration wasn't adding missing supported section markers. For example, if a file had no secrets section, the migrated file should automatically include empty secrets markers after the outputs section.

Before:

<!-- outputs:start -->
Output details
<!-- outputs:end -->
<!-- examples:start -->
Example content
<!-- examples:end -->

After:

<!-- outputs:start -->
Output details
<!-- outputs:end -->
<!-- secrets:start -->
<!-- secrets:end -->
<!-- examples:start -->
Example content
<!-- examples:end -->

4. Section Merging for Same Mappings

When multiple sections map to the same target section (e.g., both branding and title map to header), they are now merged into a single section instead of creating separate sections.

Before:

<!-- start branding -->
Brand stuff
<!-- end branding -->
<!-- start title -->
Header here
<!-- end title -->

Would become:

<!-- header:start -->
Brand stuff
<!-- header:end -->
<!-- header:start -->
Header here
<!-- header:end -->

After (New Behavior):

<!-- header:start -->
Brand stuff
Header here
<!-- header:end -->

Implementation Details

  • Fixed mapping: Updated sectionMappings in GitHubActionReadmeGeneratorMigrationAdapter
  • Removed unsupported markers: Modified AbstractMigrationAdapter.processMarkerMappings() to return empty string for unmapped sections instead of preserving original markers
  • Added missing sections: Implemented addMissingSections() method that intelligently inserts missing supported sections in appropriate positions
  • Enhanced regex patterns: Added support for hyphenated section names and fixed ESLint escaping issues
  • Section merging: Added mergeConsecutiveSections() method that detects and merges consecutive sections with the same target mapping
  • Updated documentation: Added concise documentation about section merging functionality

Testing

Added comprehensive test coverage including:

  • Validation that both branding and title correctly map to header
  • Edge case testing for unsupported marker removal while preserving content
  • Verification that missing secrets sections are automatically added after outputs
  • New: Test case specifically validating section merging behavior
  • Regression testing to ensure existing functionality remains intact

All 20 migration adapter tests pass, confirming that the changes don't break existing migration tools (actdocs, auto-doc, action-docs).

Fixes #109.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] feat(migrate): improve migrate behavior feat(migrate): improve GitHub Actions README Generator migration behavior Sep 19, 2025
Copilot AI requested a review from neilime September 19, 2025 07:55
@neilime neilime marked this pull request as ready for review September 19, 2025 09:00
Copy link
Copy Markdown
Member

@neilime neilime left a comment

Choose a reason for hiding this comment

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

@copilot when migration have several same mapping marker it should merge them into a single one.

By example:

<!-- start branding -->
Brand stuff
<!-- end branding -->
<!-- start title -->
Header here
<!-- end title -->

Should be migrated to:

<!-- header:start -->
Brand stuff
Header here
<!-- header:end -->

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 19, 2025

Codecov Report

❌ Patch coverage is 95.50562% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.56%. Comparing base (d7c033b) to head (df440ab).
⚠️ Report is 220 commits behind head on main.

Files with missing lines Patch % Lines
...ctions/src/migration/abstract-migration.adapter.ts 94.16% 7 Missing ⚠️
...c/formatter/markdown/markdown-formatter.adapter.ts 97.56% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #110      +/-   ##
==========================================
+ Coverage   86.07%   86.56%   +0.49%     
==========================================
  Files          70       70              
  Lines        4230     4303      +73     
  Branches      945      963      +18     
==========================================
+ Hits         3641     3725      +84     
+ Misses        580      569      -11     
  Partials        9        9              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 19, 2025

@copilot when migration have several same mapping marker it should merge them into a single one.

By example:...

Implemented section merging for consecutive sections with the same mapping. Now when branding and title sections are consecutive, they merge into a single header section with combined content instead of creating separate sections.

See commit 6751cf8 for the implementation with test coverage.

Copilot AI and others added 3 commits September 19, 2025 12:18
- Fix mapping: Both branding and title now map to header (was: branding->badges)
- Remove unsupported markers: Content markers with no mapping are removed while preserving content
- Add missing sections: Empty secrets section marker added after outputs section
- Update existing tests to reflect correct behavior

Co-authored-by: neilime <314088+neilime@users.noreply.github.com>
- Added comprehensive tests for edge cases
- Fixed regex pattern escaping for linter compliance
- Validate unsupported marker removal preserves content
- Validate missing secrets section insertion after outputs
- All migration adapters pass tests

Co-authored-by: neilime <314088+neilime@users.noreply.github.com>
Signed-off-by: Emilien Escalle <emilien.escalle@escemi.com>
…le section

When multiple sections map to the same target section (e.g., both 'branding' and 'title' map to 'header'),
they are now merged into a single section instead of creating separate sections.

Example:
<!-- start branding -->Brand stuff<!-- end branding -->
<!-- start title -->Header here<!-- end title -->

Now becomes:
<!-- header:start -->
Brand stuff
Header here
<!-- header:end -->

Instead of two separate header sections.

Co-authored-by: neilime <314088+neilime@users.noreply.github.com>
Signed-off-by: Emilien Escalle <emilien.escalle@escemi.com>
Copy link
Copy Markdown
Member

@neilime neilime left a comment

Choose a reason for hiding this comment

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

@copilot update documentation, be concise

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 19, 2025

@copilot update documentation, be concise

Updated migration documentation with concise section about the new section merging functionality. Added description and example showing how branding + title sections merge into a single header section.

See commit eca5026 for the documentation updates.

…rator

Add concise documentation about the new section merging functionality:
- Updated migration coverage to mention smart section merging
- Added example showing branding + title sections merging into single header
- Kept documentation concise and focused on the new feature

Co-authored-by: neilime <314088+neilime@users.noreply.github.com>
Signed-off-by: Emilien Escalle <emilien.escalle@escemi.com>
@neilime neilime merged commit 225e95a into main Sep 19, 2025
40 of 41 checks passed
@neilime neilime deleted the copilot/fix-109 branch September 19, 2025 18:50
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.

feat(migrate): improve migrate behavior

3 participants