Skip to content

Conversation

@TheMeier
Copy link

Check README for a specific comment and if found replace badge URLs based on module settings.

Refs: voxpupuli/plumbing#84

@TheMeier
Copy link
Author

TheMeier commented Dec 21, 2025

Not sure about the coveralls badge. I don't know when coveralls is available. The check for lib directory is from https://github.com/voxpupuli/modulesync_config/blob/master/moduleroot/Rakefile.erb#L35

@TheMeier
Copy link
Author

Shall I add the puppetforge badges too? https://img.shields.io/puppetforge/.....

Copy link

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 adds functionality to automatically update README badge URLs based on module settings. The feature searches for a specific placeholder comment in README.md files and replaces it with dynamically generated badges for build status, code coverage, and license information.

Key Changes:

  • Added update_readme_badges method to generate and insert badges into README files
  • Integrated badge updates into the module sync workflow to run after managed files are processed

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 119 to 141
def self.update_readme_badges(puppet_module)
return unless File.exist?(puppet_module.path('README.md'))

content = File.read(puppet_module.path('README.md'))
badge_regex = %r{^\[//\]: # \(modulesync badge placeholder, do not remove\)$}

build_status = "![Build Status](https://github.com/voxpupuli/#{puppet_module.repository_name}/actions/workflows/ci.yml/badge.svg?branch=master)\n"
code_coverage = "[![Code Coverage](https://coveralls.io/repos/github/voxpupuli/#{puppet_module.repository_name}/badge.svg?branch=master)](https://coveralls.io/github/voxpupuli/#{puppet_module.repository_name})\n"
license = "[![License](https://img.shields.io/github/license/voxpupuli/#{puppet_module.repository_name}.svg)](LICENSE)\n"

badges = build_status
badges << license
badges << code_coverage if Dir.exist?(File.expand_path('./lib', puppet_module.path))
content.gsub!(badge_regex, badges.chomp)

File.write(puppet_module.path('README.md'), content)
end
Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The new update_readme_badges method lacks test coverage. Given that the repository has existing test files for ModuleSync methods, this new method should have corresponding tests to verify its behavior, including edge cases like missing placeholder comments, various directory structures, and proper badge generation.

Copilot uses AI. Check for mistakes.
Comment on lines 125 to 126
build_status = "![Build Status](https://github.com/voxpupuli/#{puppet_module.repository_name}/actions/workflows/ci.yml/badge.svg?branch=master)\n"
code_coverage = "[![Code Coverage](https://coveralls.io/repos/github/voxpupuli/#{puppet_module.repository_name}/badge.svg?branch=master)](https://coveralls.io/github/voxpupuli/#{puppet_module.repository_name})\n"
Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The branch name 'master' is hardcoded in all badge URLs. Different repositories may use different default branch names (e.g., 'main'). Consider making the branch name configurable or detecting it from the repository settings.

Suggested change
build_status = "![Build Status](https://github.com/voxpupuli/#{puppet_module.repository_name}/actions/workflows/ci.yml/badge.svg?branch=master)\n"
code_coverage = "[![Code Coverage](https://coveralls.io/repos/github/voxpupuli/#{puppet_module.repository_name}/badge.svg?branch=master)](https://coveralls.io/github/voxpupuli/#{puppet_module.repository_name})\n"
default_branch = ENV['MODULESYNC_DEFAULT_BRANCH'] || 'master'
build_status = "![Build Status](https://github.com/voxpupuli/#{puppet_module.repository_name}/actions/workflows/ci.yml/badge.svg?branch=#{default_branch})\n"
code_coverage = "[![Code Coverage](https://coveralls.io/repos/github/voxpupuli/#{puppet_module.repository_name}/badge.svg?branch=#{default_branch})](https://coveralls.io/github/voxpupuli/#{puppet_module.repository_name})\n"

Copilot uses AI. Check for mistakes.
@TheMeier TheMeier force-pushed the plumbing/issues/84 branch 2 times, most recently from eeec172 to 6afefb4 Compare December 21, 2025 12:11
Check README for specific comments and if found render badges between them.

Refs: voxpupuli/plumbing#84
@TheMeier TheMeier marked this pull request as ready for review December 21, 2025 12:21
@smortex smortex changed the title feat: Add badge setting in README Add badge setting in README Dec 22, 2025
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I don't think this belongs in modulesync itself. This is really specific for Vox Pupuli. Not everything has code coverage, file names may be different for other repos, etc.

IMHO we should drop all badges from READMEs because they're really pointless in many cases. For published entries (like on the Forge) they will point to master but is that really correct if you look at an older release?

Instead I think we should implement a hooks mechanism in modulesync so this can live within a modulesync config.

@TheMeier
Copy link
Author

Hey @ekohl,
Actually, I totally agree with you. I was just playing around a bit. After a good night's sleep I was wondering myself if the very limited (or no) benefit of these labels actually warrant any effort at all. Regarding the hooks approach I really like that idea. Maybe I will try to make some poc over the holidays.

I will set this to draft for now so it does not accidentally merged.

@TheMeier TheMeier marked this pull request as draft December 22, 2025 16:48
@ekohl
Copy link
Member

ekohl commented Dec 22, 2025

#305 has been open for quite some time, but that would make it easier. Perhaps the path to take?

@TheMeier
Copy link
Author

Hm I did not know there was already something. But wouldn't it make more sense to have hook-implementations in modulesync_configs, possibly even with enable/disable filters on top via modulesync.yml

@ekohl
Copy link
Member

ekohl commented Dec 23, 2025

That's what #305 aims to do. Right now the hook must live within modulesync itself, but that doesn't make sense. So by making the path completely configurable you can set it in modulesync_configs to a path inside modulesync_configs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants