-
-
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 various [Polymart] badges #8352
Conversation
|
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.
Looks like the reason the service tests build is failing is because the service file is called polymart-rating.service.js
(correctly) but the corresponding tests are polymart-raiting.tester.js
(with an i
). Fixing that should allow the service tests to run.
I've left some quick comments. I'll do a proper review once we've got a build passing. Thanks
|
||
async handle({ resourceId }) { | ||
const { response } = await this.fetch({ resourceId }) | ||
return this.constructor.render({ |
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.
Could we use renderVersionBadge()
here?
Lines 144 to 150 in 302c860
function renderVersionBadge({ version, tag, defaultLabel }) { | |
return { | |
label: tag ? `${defaultLabel}@${tag}` : undefined, | |
message: addv(version), | |
color: versionColor(version), | |
} | |
} |
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.
Hello, how do you recommend adding it? It seems removed now.
services/polymart/polymart-base.js
Outdated
}), | ||
}), | ||
}), | ||
}), |
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.
All the Joi.object()
s (response
, resource
, etc) should be .required()
too as well as just the values inside them. Otherwise the objects themselves are optional.
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.
Ok, thanks for the information will add this too.
.get('/rating/323.json') | ||
.expectBadge({ | ||
label: 'rating', | ||
message: withRegex(/^(\d*\.\d+)(\/5 \()(\d+)(\))$/), |
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.
We've got a variety of service tests all defining slightly different regular expressions to do roughly the same thing e.g:
shields/services/pkgreview/package-rating.tester.js
Lines 5 to 7 in b624179
const isRatingWithReviews = withRegex( /^(([0-4](.?([0-9]))?)|5)\/5?\s*\([0-9]*\)$/ ) message: withRegex(/^(\d*\.\d+)(\/5 \()(\d+)(\))$/), const isVscodeRating = withRegex(/[0-5]\.[0-9]{1}\/5?\s*\([0-9]*\)$/)
which we should pull out into a shared helper in https://github.com/badges/shields/blob/master/services/test-validators.js
I reckon it doesn't have to be a blocker to merging this, but I'm going to at least just tag here #4902 to remind us. It might make a nice next PR to to work on if you were looking for a follow-up :)
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.
Ok, should I leave it like that or wait until you add it to the shared helper?
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.
lets leave it as it is for this PR
I've fixed that misspelling issue but there is another one |
I checked your branch out locally and tried running the tests and there's a couple of problems. One is that you are using id=1 for your "not found" tests, but this is actually a valid Resource ID on PolyMart: I'd suggest changing the 'not found' tests to use id=0 - this isn't a real resource, and it seems like a reasonably safe bet it probably won't be. Once you do, that the second problem you're going to hit is that the 'not found' response from this API isn't a
Something this would be a good example of this pattern to crib from: shields/services/steam/steam-workshop.service.js Lines 24 to 55 in 0d6873b
shields/services/steam/steam-workshop.service.js Lines 146 to 148 in 0d6873b
You can run your service tests locally using
I think the messages about missing credentials are a red herring - they apply to other services. |
@chris48s Hey, I fixed some issues, and there is a problem, while I am trying to run a test for a single service, would you mind helping me out? Here's what happens when I try running only
|
Hmm. I wonder if you are hitting #8437 Are you using Windows? |
Yep! I am using Windows, and I am probably facing the same issue. |
Bump! |
Bumps [cypress](https://github.com/cypress-io/cypress) from 12.0.2 to 12.1.0. - [Release notes](https://github.com/cypress-io/cypress/releases) - [Changelog](https://github.com/cypress-io/cypress/blob/develop/CHANGELOG.md) - [Commits](cypress-io/cypress@v12.0.2...v12.1.0) --- updated-dependencies: - dependency-name: cypress dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [sinon](https://github.com/sinonjs/sinon) from 15.0.0 to 15.0.1. - [Release notes](https://github.com/sinonjs/sinon/releases) - [Changelog](https://github.com/sinonjs/sinon/blob/main/docs/changelog.md) - [Commits](sinonjs/sinon@v15.0.0...v15.0.1) --- updated-dependencies: - dependency-name: sinon dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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>
Bumps [camelcase](https://github.com/sindresorhus/camelcase) from 7.0.0 to 7.0.1. - [Release notes](https://github.com/sindresorhus/camelcase/releases) - [Commits](sindresorhus/camelcase@v7.0.0...v7.0.1) --- updated-dependencies: - dependency-name: camelcase dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [simple-icons](https://github.com/simple-icons/simple-icons) from 8.0.0 to 8.1.0. - [Release notes](https://github.com/simple-icons/simple-icons/releases) - [Commits](simple-icons/simple-icons@8.0.0...8.1.0) --- updated-dependencies: - dependency-name: simple-icons dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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>
Bumps [@sentry/node](https://github.com/getsentry/sentry-javascript) from 7.24.2 to 7.26.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.24.2...7.26.0) --- updated-dependencies: - dependency-name: "@sentry/node" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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>
Bumps [mocha](https://github.com/mochajs/mocha) from 10.1.0 to 10.2.0. - [Release notes](https://github.com/mochajs/mocha/releases) - [Changelog](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md) - [Commits](mochajs/mocha@v10.1.0...v10.2.0) --- updated-dependencies: - dependency-name: mocha dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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 query-string from 7.1.3 to 8.0.3 Bumps [query-string](https://github.com/sindresorhus/query-string) from 7.1.3 to 8.0.3. - [Release notes](https://github.com/sindresorhus/query-string/releases) - [Commits](sindresorhus/query-string@v7.1.3...v8.0.3) --- updated-dependencies: - dependency-name: query-string dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * update import Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: chris48s <git@chris-shaw.dev>
Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com>
Bumps [@sentry/node](https://github.com/getsentry/sentry-javascript) from 7.26.0 to 7.28.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.26.0...7.28.1) --- updated-dependencies: - dependency-name: "@sentry/node" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [cypress](https://github.com/cypress-io/cypress) from 12.1.0 to 12.2.0. - [Release notes](https://github.com/cypress-io/cypress/releases) - [Changelog](https://github.com/cypress-io/cypress/blob/develop/CHANGELOG.md) - [Commits](cypress-io/cypress@v12.1.0...v12.2.0) --- updated-dependencies: - dependency-name: cypress dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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>
Bumps [@babel/core](https://github.com/babel/babel/tree/HEAD/packages/babel-core) from 7.20.5 to 7.20.7. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.20.7/packages/babel-core) --- updated-dependencies: - dependency-name: "@babel/core" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [query-string](https://github.com/sindresorhus/query-string) from 8.0.3 to 8.1.0. - [Release notes](https://github.com/sindresorhus/query-string/releases) - [Commits](sindresorhus/query-string@v8.0.3...v8.1.0) --- updated-dependencies: - dependency-name: query-string dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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>
Bumps [@babel/core](https://github.com/babel/babel/tree/HEAD/packages/babel-core) from 7.20.7 to 7.20.12. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.20.12/packages/babel-core) --- updated-dependencies: - dependency-name: "@babel/core" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [json5](https://github.com/json5/json5) - [Release notes](https://github.com/json5/json5/releases) - [Changelog](https://github.com/json5/json5/blob/main/CHANGELOG.md) - [Commits](json5/json5@v1.0.1...v1.0.2) --- updated-dependencies: - dependency-name: json5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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>
* migrate danger CI job to GHA * constrain triggers * prettier
* add docstrings for pipenv helpers * update param description Co-authored-by: chris48s <chris48s@users.noreply.github.com>
@chris48s Bump |
Bumps [prettier](https://github.com/prettier/prettier) from 2.8.1 to 2.8.2. - [Release notes](https://github.com/prettier/prettier/releases) - [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md) - [Commits](prettier/prettier@2.8.1...2.8.2) --- updated-dependencies: - dependency-name: prettier dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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>
Sorry this one has been on a back burner for a while. Looks like you've still got some failing tests but we merged #8786 recently which should allow you to update this branch and run your tests locally. Can you have a look at getting the build passing and I'll circle back and review. |
Bumps [@sentry/node](https://github.com/getsentry/sentry-javascript) from 7.29.0 to 7.30.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.29.0...7.30.0) --- updated-dependencies: - dependency-name: "@sentry/node" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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>
Bumps [rimraf](https://github.com/isaacs/rimraf) from 3.0.2 to 4.0.4. - [Release notes](https://github.com/isaacs/rimraf/releases) - [Changelog](https://github.com/isaacs/rimraf/blob/main/CHANGELOG.md) - [Commits](isaacs/rimraf@v3.0.2...v4.0.4) --- updated-dependencies: - dependency-name: rimraf dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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>
…ges#8807) Bumps [eslint-plugin-import](https://github.com/import-js/eslint-plugin-import) from 2.26.0 to 2.27.4. - [Release notes](https://github.com/import-js/eslint-plugin-import/releases) - [Changelog](https://github.com/import-js/eslint-plugin-import/blob/main/CHANGELOG.md) - [Commits](import-js/eslint-plugin-import@v2.26.0...v2.27.4) --- updated-dependencies: - dependency-name: eslint-plugin-import dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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>
…ges#8798) Bumps [eslint-plugin-react](https://github.com/jsx-eslint/eslint-plugin-react) from 7.31.11 to 7.32.0. - [Release notes](https://github.com/jsx-eslint/eslint-plugin-react/releases) - [Changelog](https://github.com/jsx-eslint/eslint-plugin-react/blob/master/CHANGELOG.md) - [Commits](jsx-eslint/eslint-plugin-react@v7.31.11...v7.32.0) --- updated-dependencies: - dependency-name: eslint-plugin-react dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [fast-xml-parser](https://github.com/NaturalIntelligence/fast-xml-parser) from 4.0.12 to 4.0.13. - [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.12...v4.0.13) --- updated-dependencies: - dependency-name: fast-xml-parser dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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>
Bumps [nock](https://github.com/nock/nock) from 13.2.9 to 13.3.0. - [Release notes](https://github.com/nock/nock/releases) - [Changelog](https://github.com/nock/nock/blob/main/CHANGELOG.md) - [Commits](nock/nock@v13.2.9...v13.3.0) --- updated-dependencies: - dependency-name: nock dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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>
…s#8802) Bumps [@renovatebot/ruby-semver](https://github.com/renovatebot/ruby-semver) from 1.1.7 to 1.1.8. - [Release notes](https://github.com/renovatebot/ruby-semver/releases) - [Changelog](https://github.com/renovatebot/ruby-semver/blob/main/.releaserc.json) - [Commits](renovatebot/ruby-semver@1.1.7...1.1.8) --- updated-dependencies: - dependency-name: "@renovatebot/ruby-semver" dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> 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: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com>
…elds into polymart-badges
Error Error
Dangerfile
|
Hey! Thank you for that, could you please tell me if there is a problem with my latest activity, because I feel like I've done something wrong. |
Yes there's definitely something wrong with this branch now, as we can tell from the diff: https://github.com/badges/shields/pull/8352/files Rather than try to unpick it, I think from this point the best suggestion I can give you is: I'm assuming you have https://github.com/badges/shields.git set up as a remote called Pull down the latest changes:
Create a clean branch based off the latest changes from the upstream:
Cherry pick the commits you want from this branch onto the new one:
That will give you a new branch based off the latest commit on master with you changes so far from this PR replayed on top of it that you can work from. Then either submit a new PR from that branch and I'll pick up the review from there, or force push your new branch to this one. |
Thank you very much, I will open a new pull request! |
Added:
Downloads
badgeLatest Version
badgeRating
badgeStar Rating
badgeCloses #7429