-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Add [ROS] version service #8169
Conversation
Thanks for opening a PR! To the best of my knowledge, we don't have any open issues requesting this badge with the relevant details, so before we proceed it would be helpful for us if you could speak to some of the questions we ask in the issue template for new badges so that we've a sufficient amount of context (I've not heard of ROS myself) I think the first several are implicitly covered already, so I've taken a pass at filling them in, but certainly feel free to correct.
|
I've also shared a link to this PR in the ROS Discourse to notify the maintainers and request any feedback they might have: https://discourse.ros.org/t/shields-io-badge-service/26379 |
Hi @calebcartwright, just to follow up on this, is there any more info I can provide? |
No, the info you've provided is great thanks! Someone should get around to reviewing this in the not too distant future |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting this and including tests. It is looking in pretty good shape to start with and its clear you've worked through the documentation and contributing guidelines 👍
services/ros/ros-version.service.js
Outdated
packageName | ||
) | ||
|
||
return { ...renderVersionBadge({ version }), label: distro } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah agreed 👍 Change it on this PR and I will do another PR to update the conda one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
This is normal - I wouldn't worry about it. We probably should set the
We could find a way to do this, but tbh I think the approach you've already used of doing both queries using GraphQL is fine and maybe the neatest solution anyway. I don't think we need to change it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Add [ROS] version service * review feedback * add spaces
* Add greasy fork rating badges * refactor: combine rating classes * refactor: remove Base from class name * Change to a single badge with all values * Add unit tests for GreasyForkRatingCount.render * Average totals in color algorithm * chore(deps): bump moment from 2.29.3 to 2.29.4 (#8170) Bumps [moment](https://github.com/moment/moment) from 2.29.3 to 2.29.4. - [Release notes](https://github.com/moment/moment/releases) - [Changelog](https://github.com/moment/moment/blob/develop/CHANGELOG.md) - [Commits](moment/moment@2.29.3...2.29.4) --- updated-dependencies: - dependency-name: moment dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com> * chore(deps-dev): bump @typescript-eslint/eslint-plugin (#8183) Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 5.30.0 to 5.30.5. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v5.30.5/packages/eslint-plugin) --- updated-dependencies: - dependency-name: "@typescript-eslint/eslint-plugin" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com> * chore(deps): bump @sentry/node from 7.4.1 to 7.5.1 (#8174) Bumps [@sentry/node](https://github.com/getsentry/sentry-javascript) from 7.4.1 to 7.5.1. - [Release notes](https://github.com/getsentry/sentry-javascript/releases) - [Changelog](https://github.com/getsentry/sentry-javascript/blob/master/CHANGELOG.md) - [Commits](getsentry/sentry-javascript@7.4.1...7.5.1) --- updated-dependencies: - dependency-name: "@sentry/node" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com> * chore(deps): bump simple-icons from 7.3.0 to 7.4.0 (#8181) Bumps [simple-icons](https://github.com/simple-icons/simple-icons) from 7.3.0 to 7.4.0. - [Release notes](https://github.com/simple-icons/simple-icons/releases) - [Commits](simple-icons/simple-icons@7.3.0...7.4.0) --- updated-dependencies: - dependency-name: simple-icons dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com> * chore(deps-dev): bump nodemon from 2.0.18 to 2.0.19 (#8179) Bumps [nodemon](https://github.com/remy/nodemon) from 2.0.18 to 2.0.19. - [Release notes](https://github.com/remy/nodemon/releases) - [Commits](remy/nodemon@v2.0.18...v2.0.19) --- updated-dependencies: - dependency-name: nodemon dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com> * Add [ROS] version service (#8169) * Add [ROS] version service * review feedback * add spaces * add spaces round pipe in [conda] badge (#8189) Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com> * refactor(deps): Replace moment with dayjs (#8192) * chore(deps): bump @sentry/node from 7.5.1 to 7.6.0 (#8197) Bumps [@sentry/node](https://github.com/getsentry/sentry-javascript) from 7.5.1 to 7.6.0. - [Release notes](https://github.com/getsentry/sentry-javascript/releases) - [Changelog](https://github.com/getsentry/sentry-javascript/blob/master/CHANGELOG.md) - [Commits](getsentry/sentry-javascript@7.5.1...7.6.0) --- updated-dependencies: - dependency-name: "@sentry/node" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix missing `dayjs` -> `moment` (#8204) Co-authored-by: Caleb Cartwright <[email protected]> * chore(deps): bump ioredis from 5.1.0 to 5.2.0 (#8201) Bumps [ioredis](https://github.com/luin/ioredis) from 5.1.0 to 5.2.0. - [Release notes](https://github.com/luin/ioredis/releases) - [Changelog](https://github.com/luin/ioredis/blob/main/CHANGELOG.md) - [Commits](redis/ioredis@v5.1.0...v5.2.0) --- updated-dependencies: - dependency-name: ioredis dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com> * chore(deps): bump fast-xml-parser from 4.0.8 to 4.0.9 (#8203) Bumps [fast-xml-parser](https://github.com/NaturalIntelligence/fast-xml-parser) from 4.0.8 to 4.0.9. - [Release notes](https://github.com/NaturalIntelligence/fast-xml-parser/releases) - [Changelog](https://github.com/NaturalIntelligence/fast-xml-parser/blob/master/CHANGELOG.md) - [Commits](NaturalIntelligence/fast-xml-parser@v4.0.8...v4.0.9) --- updated-dependencies: - dependency-name: fast-xml-parser dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * revert rebase * Add test for all good ratings Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com> Co-authored-by: Jacob Bandes-Storch <[email protected]> Co-authored-by: chris48s <[email protected]> Co-authored-by: chase <[email protected]> Co-authored-by: Caleb Cartwright <[email protected]>
Adds version service at
/ros/v/:distro/:packageName
for ROS packages. Version information is loaded directly from the ros/rosdistro repo on GitHub, using tags to determine the latest available distribution, and then parsing the distribution.yaml file from that distribution.I was hoping to make the
:distro
show up as the badgelabel
in error cases, but it seems like when an error is thrown, only the defaultBadgeLabel (or'version'
) is used as the label. And I would've happily used theraw.githubusercontent.com
method of loading the distribution.yaml file, but it didn't seem like one service would be able to make both GraphQL requests and REST requests (when I tried to make a REST request, it still prepended the GraphQL endpoint's base URL). Any advice on these would be appreciated!