Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor - use renderVersionBadge - part 3 [luarocks gitlab nuget feedz] #10630

Merged
merged 8 commits into from
Oct 27, 2024

Conversation

jNullj
Copy link
Member

@jNullj jNullj commented Oct 24, 2024

This pull request refactors the codebase to use the renderVersionBadge function for rendering version badges.
Refactored services are luarocks, gitlab, feedz and nuget.
The pull request also modifies tests where needed.
This change is part of the ongoing work to address issue #2026 - refactor all versions to use renderVersionBadge
And also helps archive user override for version prefix as mentioned in #10574

Additionally, the scm and cvs versions are now included as previews in the renderVersionBadge function project-wide.
And a comment was added to clojars to make it clear it uses non-standard style while pointing to relevant PR and commit.

The only services left for the next part are found using this regex first to see if all imports belong to version.js
imports import\s*(?:[ \n\t]*(?:[^ \n\t\{\}]+[ \n\t]*,?)?\s*\{[^}]*\brenderVersionBadge\b[^}]*\})?\s*from\s*(['"])([^'"\n]+)\1
Then i used grep -rl "category = 'version'" . | xargs grep -L "renderVersionBadge" to find all missing renderVersionBadge usages for badges in the versions category.

which only leaves

./services/visual-studio-app-center/visual-studio-app-center-releases-osversion.service.js
./services/github/github-release.service.js
./services/github/github-tag.service.js

visual-studio-app-center-releases-osversion uses os minimum versions so its not relevant.

@jNullj jNullj changed the title Refactor - usage renderVersionBadge - part 3 [luarocks gitlab nuget feedz] Refactor - use renderVersionBadge - part 3 [luarocks gitlab nuget feedz] Oct 24, 2024
@jNullj jNullj added the javascript [dependabot only] Pull requests that update Javascript packages label Oct 24, 2024
Copy link
Contributor

github-actions bot commented Oct 24, 2024

Warnings
⚠️ This PR modified service code for clojars but not its test code.
That's okay so long as it's refactoring existing code.
⚠️ This PR modified service code for gitlab but not its test code.
That's okay so long as it's refactoring existing code.
⚠️ This PR modified service code for luarocks but not its test code.
That's okay so long as it's refactoring existing code.
Messages
📖 ✨ Thanks for your contribution to Shields, @jNullj!

Generated by 🚫 dangerJS against e78cc40

@jNullj jNullj added developer-experience Dev tooling, test framework, and CI core Server, BaseService, GitHub auth, Shared helpers labels Oct 24, 2024
Copy link
Member

@PyvesB PyvesB left a comment

Choose a reason for hiding this comment

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

Changes look good to me! Switching to renderVersionBadge will modify colors in some cases, but I don't think the colors in these badges were ever motivated by vertical consistency with the upstream service; consequently improving horizontal consistency across Shields.io badges is a good thing.

@PyvesB PyvesB added this pull request to the merge queue Oct 27, 2024
Merged via the queue into badges:master with commit cc90c19 Oct 27, 2024
23 checks passed
@robaho
Copy link

robaho commented Jan 15, 2025

Hi @PyvesB - is there a reason maven-central was not included in the list of services that should be updated to use renderVersionBadge?

@jNullj
Copy link
Member Author

jNullj commented Jan 15, 2025

Hi @PyvesB - is there a reason maven-central was not included in the list of services that should be updated to use renderVersionBadge?

maven-central is a redirector to maven-metadata which uses renderVersionBadge since 2021.

@robaho
Copy link

robaho commented Jan 16, 2025

@jNullj That doesn't appear to be the case in the code repo, but regardless, I tried using maven-metadata and still the prefix is not passed through and rendered. I think this is the correct url:

https://img.shields.io/maven-metadata/v?metadataUrl=https://repo1.maven.org/maven2/io/github/robaho/httpserver/maven-metadata.xml&prefix=X

It is rendered with the v not the X.

@jNullj
Copy link
Member Author

jNullj commented Jan 16, 2025

@robaho Thank you for clarifying, I think i better understand now what you find missing.
This PR only adds the prefix param to renderVersionBadge, this is internal code. For the prefix to show as a badge param we still need to add it to the badges.
I did an attempt at #10685 but ended up closing the PR.
To better understand why prefix was not added please make a quick read for that PR.
Please continue discussion regarding the prefix feature in the related PR/issues, as this PR is for making the code more consistent and easier to maintain and not for adding the new user facing feature, this is a refactor, i only mentioned it here as i made it easier to implement in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Server, BaseService, GitHub auth, Shared helpers developer-experience Dev tooling, test framework, and CI javascript [dependabot only] Pull requests that update Javascript packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants