Skip to content

Add badge setting in README#337

Closed
TheMeier wants to merge 1 commit into
masterfrom
plumbing/issues/84
Closed

Add badge setting in README#337
TheMeier wants to merge 1 commit into
masterfrom
plumbing/issues/84

Conversation

@TheMeier

Copy link
Copy Markdown

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

Refs: voxpupuli/plumbing#84

@TheMeier

TheMeier commented Dec 21, 2025

Copy link
Copy Markdown
Author

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
Copy Markdown
Author

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

Copilot AI 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.

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 thread lib/modulesync.rb Outdated
Comment thread lib/modulesync.rb
Comment on lines +119 to +135
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

Copilot AI Dec 21, 2025

Copy link

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 thread lib/modulesync.rb Outdated
Comment thread lib/modulesync.rb Outdated
Comment thread lib/modulesync.rb Outdated
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"

Copilot AI Dec 21, 2025

Copy link

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.
Comment thread lib/modulesync.rb Outdated
@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

@ekohl ekohl left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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

ekohl commented Dec 22, 2025

Copy link
Copy Markdown
Member

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

@TheMeier

Copy link
Copy Markdown
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

ekohl commented Dec 23, 2025

Copy link
Copy Markdown
Member

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.

@TheMeier TheMeier closed this Jan 25, 2026
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.

3 participants